From owner-svn-src-head@freebsd.org Thu Sep 8 17:40:41 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DA03BBD1A79; Thu, 8 Sep 2016 17:40:41 +0000 (UTC) (envelope-from bde@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 9ECDB231; Thu, 8 Sep 2016 17:40:41 +0000 (UTC) (envelope-from bde@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u88HeeBH044376; Thu, 8 Sep 2016 17:40:40 GMT (envelope-from bde@FreeBSD.org) Received: (from bde@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u88HeeRK044371; Thu, 8 Sep 2016 17:40:40 GMT (envelope-from bde@FreeBSD.org) Message-Id: <201609081740.u88HeeRK044371@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: bde set sender to bde@FreeBSD.org using -f From: Bruce Evans Date: Thu, 8 Sep 2016 17:40:40 +0000 (UTC) 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 X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Sep 2016 17:40:41 -0000 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