From 0c01bcb9ffd9168f8fe5cb786c85d47437f0606a Mon Sep 17 00:00:00 2001 From: Bruce Evans Date: Thu, 8 Sep 2016 17:40:40 +0000 Subject: [PATCH] Sprinkle DOINGASYNC() checks so as to do delayed writes for async mounts in almost all cases instead of in most cases. Don't override DOINGASYNC() by any condition except IO_SYNC. Fix previous sprinking of DOINGASYNC() checks. Don't override IO_SYNC by DOINGASYNC(). In ffs_write() and ffs_extwrite(), there were intentional overrides that just broke O_SYNC of data. In ffs_truncate(), there are 5 calls to ffs_update(), 4 with apparently-unintentional overrides and 1 without; this had no effect due to the main async mount hack descibed below. Fix 1 place in ffs_truncate() where the caller's IO_ASYNC was overridden for the soft updates case too (to do a delayed write instead of a sync write). This is supposed to be the only change that affects anything except async mounts. In ffs_update(), remove the 19 year old efficiency hack of ignoring the waitfor flag for async mounts, so that fsync() almost works for async mounts. All callers are supposed to be fixed to not ask for a sync update unless they are for fsync() or [I]O_SYNC operations. fsync() now almost works for async mounts. It used to sync the data but not the most important metdata (the inode). It still doesn't sync associated directories. This gave 10-20% fewer writes for my makeworld benchmark with async mounted tmp and obj directories from an already small number. Style fixes: - in ffs_balloc.c, remove rotted quadruplicated comments about the simplest part of the DOING*() decisions and rearrange the nearly- quadruplicated code to be more nearly so. - in ufs_vnops.c, use a consistent style with less negative logic and no manual "optimization" of || to | in DOING*() expressions. Reviewed by: kib (previous version) --- sys/ufs/ffs/ffs_balloc.c | 42 +++++++++++++++++++--------------------- sys/ufs/ffs/ffs_inode.c | 21 ++++++++++---------- sys/ufs/ffs/ffs_vnops.c | 4 ++-- sys/ufs/ufs/ufs_lookup.c | 2 +- sys/ufs/ufs/ufs_vnops.c | 20 +++++++++---------- 5 files changed, 44 insertions(+), 45 deletions(-) diff --git a/sys/ufs/ffs/ffs_balloc.c b/sys/ufs/ffs/ffs_balloc.c index 04e0ae93b23..676dcb4e4d4 100644 --- a/sys/ufs/ffs/ffs_balloc.c +++ b/sys/ufs/ffs/ffs_balloc.c @@ -154,6 +154,8 @@ ffs_balloc_ufs1(struct vnode *vp, off_t startoffset, int size, ip->i_flag |= IN_CHANGE | IN_UPDATE; if (flags & IO_SYNC) bwrite(bp); + else if (DOINGASYNC(vp)) + bdwrite(bp); else bawrite(bp); } @@ -266,14 +268,12 @@ ffs_balloc_ufs1(struct vnode *vp, off_t startoffset, int size, softdep_setup_allocdirect(ip, NDADDR + indirs[0].in_off, newb, 0, fs->fs_bsize, 0, bp); bdwrite(bp); + } else if ((flags & IO_SYNC) == 0 && DOINGASYNC(vp)) { + if (bp->b_bufsize == fs->fs_bsize) + bp->b_flags |= B_CLUSTEROK; + bdwrite(bp); } else { - /* - * Write synchronously so that indirect blocks - * never point at garbage. - */ - if (DOINGASYNC(vp)) - bdwrite(bp); - else if ((error = bwrite(bp)) != 0) + if ((error = bwrite(bp)) != 0) goto fail; } allocib = &dp->di_ib[indirs[0].in_off]; @@ -338,11 +338,11 @@ retry: softdep_setup_allocindir_meta(nbp, ip, bp, indirs[i - 1].in_off, nb); bdwrite(nbp); + } else if ((flags & IO_SYNC) == 0 && DOINGASYNC(vp)) { + if (nbp->b_bufsize == fs->fs_bsize) + nbp->b_flags |= B_CLUSTEROK; + bdwrite(nbp); } else { - /* - * Write synchronously so that indirect blocks - * never point at garbage. - */ if ((error = bwrite(nbp)) != 0) { brelse(bp); goto fail; @@ -854,14 +854,12 @@ ffs_balloc_ufs2(struct vnode *vp, off_t startoffset, int size, softdep_setup_allocdirect(ip, NDADDR + indirs[0].in_off, newb, 0, fs->fs_bsize, 0, bp); bdwrite(bp); + } else if ((flags & IO_SYNC) == 0 && DOINGASYNC(vp)) { + if (bp->b_bufsize == fs->fs_bsize) + bp->b_flags |= B_CLUSTEROK; + bdwrite(bp); } else { - /* - * Write synchronously so that indirect blocks - * never point at garbage. - */ - if (DOINGASYNC(vp)) - bdwrite(bp); - else if ((error = bwrite(bp)) != 0) + if ((error = bwrite(bp)) != 0) goto fail; } allocib = &dp->di_ib[indirs[0].in_off]; @@ -927,11 +925,11 @@ retry: softdep_setup_allocindir_meta(nbp, ip, bp, indirs[i - 1].in_off, nb); bdwrite(nbp); + } else if ((flags & IO_SYNC) == 0 && DOINGASYNC(vp)) { + if (nbp->b_bufsize == fs->fs_bsize) + nbp->b_flags |= B_CLUSTEROK; + bdwrite(nbp); } else { - /* - * Write synchronously so that indirect blocks - * never point at garbage. - */ if ((error = bwrite(nbp)) != 0) { brelse(bp); goto fail; diff --git a/sys/ufs/ffs/ffs_inode.c b/sys/ufs/ffs/ffs_inode.c index 50b456bd787..4bd38d58692 100644 --- a/sys/ufs/ffs/ffs_inode.c +++ b/sys/ufs/ffs/ffs_inode.c @@ -154,7 +154,7 @@ loop: /* XXX: FIX? The entropy here is desirable, but the harvesting may be expensive */ random_harvest_queue(&(ip->i_din2), sizeof(ip->i_din2), 1, RANDOM_FS_ATIME); } - if (waitfor && !DOINGASYNC(vp)) + if (waitfor) error = bwrite(bp); else if (vm_page_count_severe() || buf_dirty_count_severe()) { bawrite(bp); @@ -193,7 +193,7 @@ ffs_truncate(vp, length, flags, cred) int softdeptrunc, journaltrunc; int needextclean, extblocks; int offset, size, level, nblocks; - int i, error, allerror, indiroff; + int i, error, allerror, indiroff, waitforupdate; off_t osize; ip = VTOI(vp); @@ -221,6 +221,7 @@ ffs_truncate(vp, length, flags, cred) flags |= IO_NORMAL; if (!DOINGSOFTDEP(vp) && !DOINGASYNC(vp)) flags |= IO_SYNC; + waitforupdate = (flags & IO_SYNC) != 0 || !DOINGASYNC(vp); /* * If we are truncating the extended-attributes, and cannot * do it with soft updates, then do it slowly here. If we are @@ -264,7 +265,7 @@ ffs_truncate(vp, length, flags, cred) ip->i_din2->di_extb[i] = 0; } ip->i_flag |= IN_CHANGE; - if ((error = ffs_update(vp, !DOINGASYNC(vp)))) + if ((error = ffs_update(vp, waitforupdate))) return (error); for (i = 0; i < NXADDR; i++) { if (oldblks[i] == 0) @@ -290,7 +291,7 @@ ffs_truncate(vp, length, flags, cred) ip->i_flag |= IN_CHANGE | IN_UPDATE; if (needextclean) goto extclean; - return (ffs_update(vp, !DOINGASYNC(vp))); + return (ffs_update(vp, waitforupdate)); } if (ip->i_size == length) { ip->i_flag |= IN_CHANGE | IN_UPDATE; @@ -328,7 +329,7 @@ ffs_truncate(vp, length, flags, cred) else bawrite(bp); ip->i_flag |= IN_CHANGE | IN_UPDATE; - return (ffs_update(vp, !DOINGASYNC(vp))); + return (ffs_update(vp, waitforupdate)); } /* * Lookup block number for a given offset. Zero length files @@ -357,10 +358,10 @@ ffs_truncate(vp, length, flags, cred) */ if (blkno != 0) brelse(bp); - else if (DOINGSOFTDEP(vp) || DOINGASYNC(vp)) - bdwrite(bp); - else + else if (flags & IO_SYNC) bwrite(bp); + else + bdwrite(bp); } /* * If the block number at the new end of the file is zero, @@ -478,7 +479,7 @@ ffs_truncate(vp, length, flags, cred) DIP_SET(ip, i_db[i], 0); } ip->i_flag |= IN_CHANGE | IN_UPDATE; - allerror = ffs_update(vp, !DOINGASYNC(vp)); + allerror = ffs_update(vp, waitforupdate); /* * Having written the new inode to disk, save its new configuration @@ -610,7 +611,7 @@ extclean: softdep_journal_freeblocks(ip, cred, length, IO_EXT); else softdep_setup_freeblocks(ip, length, IO_EXT); - return (ffs_update(vp, (flags & IO_SYNC) != 0 || !DOINGASYNC(vp))); + return (ffs_update(vp, waitforupdate)); } /* diff --git a/sys/ufs/ffs/ffs_vnops.c b/sys/ufs/ffs/ffs_vnops.c index 27654804046..0ed7c592398 100644 --- a/sys/ufs/ffs/ffs_vnops.c +++ b/sys/ufs/ffs/ffs_vnops.c @@ -757,7 +757,7 @@ ffs_write(ap) flags = BA_SEQMAX << BA_SEQSHIFT; else flags = seqcount << BA_SEQSHIFT; - if ((ioflag & IO_SYNC) && !DOINGASYNC(vp)) + if (ioflag & IO_SYNC) flags |= IO_SYNC; flags |= BA_UNMAPPED; @@ -1077,7 +1077,7 @@ ffs_extwrite(struct vnode *vp, struct uio *uio, int ioflag, struct ucred *ucred) resid = uio->uio_resid; osize = dp->di_extsize; flags = IO_EXT; - if ((ioflag & IO_SYNC) && !DOINGASYNC(vp)) + if (ioflag & IO_SYNC) flags |= IO_SYNC; for (error = 0; uio->uio_resid > 0;) { diff --git a/sys/ufs/ufs/ufs_lookup.c b/sys/ufs/ufs/ufs_lookup.c index c3e3de9e878..343162434eb 100644 --- a/sys/ufs/ufs/ufs_lookup.c +++ b/sys/ufs/ufs/ufs_lookup.c @@ -1237,7 +1237,7 @@ out: } else { if (flags & DOWHITEOUT) error = bwrite(bp); - else if (DOINGASYNC(dvp) && dp->i_count != 0) + else if (DOINGASYNC(dvp)) bdwrite(bp); else error = bwrite(bp); diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c index 07e59aa1ce1..46e5b24a36f 100644 --- a/sys/ufs/ufs/ufs_vnops.c +++ b/sys/ufs/ufs/ufs_vnops.c @@ -991,7 +991,7 @@ ufs_link(ap) ip->i_flag |= IN_CHANGE; if (DOINGSOFTDEP(vp)) softdep_setup_link(VTOI(tdvp), ip); - error = UFS_UPDATE(vp, !(DOINGSOFTDEP(vp) | DOINGASYNC(vp))); + error = UFS_UPDATE(vp, !DOINGSOFTDEP(vp) && !DOINGASYNC(vp)); if (!error) { ufs_makedirentry(ip, cnp, &newdir); error = ufs_direnter(tdvp, vp, &newdir, cnp, NULL, 0); @@ -1335,7 +1335,7 @@ relock: fip->i_flag |= IN_CHANGE; if (DOINGSOFTDEP(fvp)) softdep_setup_link(tdp, fip); - error = UFS_UPDATE(fvp, !(DOINGSOFTDEP(fvp) | DOINGASYNC(fvp))); + error = UFS_UPDATE(fvp, !DOINGSOFTDEP(fvp) && !DOINGASYNC(fvp)); if (error) goto bad; @@ -1488,8 +1488,8 @@ relock: tdp->i_flag |= IN_CHANGE; if (DOINGSOFTDEP(tdvp)) softdep_setup_dotdot_link(tdp, fip); - error = UFS_UPDATE(tdvp, !(DOINGSOFTDEP(tdvp) | - DOINGASYNC(tdvp))); + error = UFS_UPDATE(tdvp, !DOINGSOFTDEP(tdvp) && + !DOINGASYNC(tdvp)); /* Don't go to bad here as the new link exists. */ if (error) goto unlockout; @@ -1529,8 +1529,8 @@ unlockout: * are no longer needed. */ if (error == 0 && endoff != 0) { - error = UFS_TRUNCATE(tdvp, endoff, IO_NORMAL | IO_SYNC, - tcnp->cn_cred); + error = UFS_TRUNCATE(tdvp, endoff, IO_NORMAL | + (DOINGASYNC(tdvp) ? 0 : IO_SYNC), tcnp->cn_cred); if (error != 0) vn_printf(tdvp, "ufs_rename: failed to truncate " "err %d", error); @@ -1888,7 +1888,7 @@ ufs_mkdir(ap) dp->i_flag |= IN_CHANGE; if (DOINGSOFTDEP(dvp)) softdep_setup_mkdir(dp, ip); - error = UFS_UPDATE(dvp, !(DOINGSOFTDEP(dvp) | DOINGASYNC(dvp))); + error = UFS_UPDATE(dvp, !DOINGSOFTDEP(dvp) && !DOINGASYNC(dvp)); if (error) goto bad; #ifdef MAC @@ -1945,8 +1945,8 @@ ufs_mkdir(ap) blkoff += DIRBLKSIZ; } } - if ((error = UFS_UPDATE(tvp, !(DOINGSOFTDEP(tvp) | - DOINGASYNC(tvp)))) != 0) { + if ((error = UFS_UPDATE(tvp, !DOINGSOFTDEP(tvp) && + !DOINGASYNC(tvp))) != 0) { (void)bwrite(bp); goto bad; } @@ -2680,7 +2680,7 @@ ufs_makeinode(mode, dvp, vpp, cnp) /* * Make sure inode goes to disk before directory entry. */ - error = UFS_UPDATE(tvp, !(DOINGSOFTDEP(tvp) | DOINGASYNC(tvp))); + error = UFS_UPDATE(tvp, !DOINGSOFTDEP(tvp) && !DOINGASYNC(tvp)); if (error) goto bad; #ifdef MAC