Date: Thu, 8 Sep 2016 17:40:40 +0000 (UTC) From: Bruce Evans <bde@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r305622 - in head/sys/ufs: ffs ufs Message-ID: <201609081740.u88HeeRK044371@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bde Date: Thu Sep 8 17:40:40 2016 New Revision: 305622 URL: https://svnweb.freebsd.org/changeset/base/305622 Log: 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) Modified: head/sys/ufs/ffs/ffs_balloc.c head/sys/ufs/ffs/ffs_inode.c head/sys/ufs/ffs/ffs_vnops.c head/sys/ufs/ufs/ufs_lookup.c head/sys/ufs/ufs/ufs_vnops.c Modified: head/sys/ufs/ffs/ffs_balloc.c ============================================================================== --- head/sys/ufs/ffs/ffs_balloc.c Thu Sep 8 17:37:13 2016 (r305621) +++ head/sys/ufs/ffs/ffs_balloc.c Thu Sep 8 17:40:40 2016 (r305622) @@ -154,6 +154,8 @@ ffs_balloc_ufs1(struct vnode *vp, off_t 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 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 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; Modified: head/sys/ufs/ffs/ffs_inode.c ============================================================================== --- head/sys/ufs/ffs/ffs_inode.c Thu Sep 8 17:37:13 2016 (r305621) +++ head/sys/ufs/ffs/ffs_inode.c Thu Sep 8 17:40:40 2016 (r305622) @@ -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)); } /* Modified: head/sys/ufs/ffs/ffs_vnops.c ============================================================================== --- head/sys/ufs/ffs/ffs_vnops.c Thu Sep 8 17:37:13 2016 (r305621) +++ head/sys/ufs/ffs/ffs_vnops.c Thu Sep 8 17:40:40 2016 (r305622) @@ -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 ui 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;) { Modified: head/sys/ufs/ufs/ufs_lookup.c ============================================================================== --- head/sys/ufs/ufs/ufs_lookup.c Thu Sep 8 17:37:13 2016 (r305621) +++ head/sys/ufs/ufs/ufs_lookup.c Thu Sep 8 17:40:40 2016 (r305622) @@ -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); Modified: head/sys/ufs/ufs/ufs_vnops.c ============================================================================== --- head/sys/ufs/ufs/ufs_vnops.c Thu Sep 8 17:37:13 2016 (r305621) +++ head/sys/ufs/ufs/ufs_vnops.c Thu Sep 8 17:40:40 2016 (r305622) @@ -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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201609081740.u88HeeRK044371>