From owner-svn-src-all@FreeBSD.ORG Tue Jan 22 07:22:58 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id CD7571E2; Tue, 22 Jan 2013 07:22:58 +0000 (UTC) (envelope-from scottl@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) by mx1.freebsd.org (Postfix) with ESMTP id A6CA4963; Tue, 22 Jan 2013 07:22:58 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.5/8.14.5) with ESMTP id r0M7MwTg056592; Tue, 22 Jan 2013 07:22:58 GMT (envelope-from scottl@svn.freebsd.org) Received: (from scottl@localhost) by svn.freebsd.org (8.14.5/8.14.5/Submit) id r0M7Mw81056591; Tue, 22 Jan 2013 07:22:58 GMT (envelope-from scottl@svn.freebsd.org) Message-Id: <201301220722.r0M7Mw81056591@svn.freebsd.org> From: Scott Long Date: Tue, 22 Jan 2013 07:22:58 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r245776 - stable/9/sys/ufs/ffs X-SVN-Group: stable-9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Jan 2013 07:22:58 -0000 Author: scottl Date: Tue Jan 22 07:22:58 2013 New Revision: 245776 URL: http://svnweb.freebsd.org/changeset/base/245776 Log: MFC r242924: - Fix a bug that has existed since the original softdep implementation. When a background copy of a cg is written we complete any work associated with that bmsafemap. If new work has been added to the non-background copy of the buffer it will be completed before the next write happens. The solution is to do the rollbacks when we make the copy so only those dependencies that were present at the time of writing will be completed when the background write completes. This would've resulted in various bitmap related corruptions and panics. It also would've expired journal entries early causing journal replay to miss some records. Modified: stable/9/sys/ufs/ffs/ffs_softdep.c Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/ufs/ffs/ffs_softdep.c ============================================================================== --- stable/9/sys/ufs/ffs/ffs_softdep.c Tue Jan 22 07:18:33 2013 (r245775) +++ stable/9/sys/ufs/ffs/ffs_softdep.c Tue Jan 22 07:22:58 2013 (r245776) @@ -976,7 +976,7 @@ static struct freework *newfreework(stru struct freework *, ufs_lbn_t, ufs2_daddr_t, int, int, int); static int jwait(struct worklist *, int); static struct inodedep *inodedep_lookup_ip(struct inode *); -static int bmsafemap_rollbacks(struct bmsafemap *); +static int bmsafemap_backgroundwrite(struct bmsafemap *, struct buf *); static struct freefile *handle_bufwait(struct inodedep *, struct workhead *); static void handle_jwork(struct workhead *); static struct mkdir *setup_newdir(struct diradd *, ino_t, ino_t, struct buf *, @@ -1800,7 +1800,7 @@ softdep_move_dependencies(oldbp, newbp) while ((wk = LIST_FIRST(&oldbp->b_dep)) != NULL) { LIST_REMOVE(wk, wk_list); if (wk->wk_type == D_BMSAFEMAP && - bmsafemap_rollbacks(WK_BMSAFEMAP(wk))) + bmsafemap_backgroundwrite(WK_BMSAFEMAP(wk), newbp)) dirty = 1; if (wktail == 0) LIST_INSERT_HEAD(&newbp->b_dep, wk, wk_list); @@ -5178,9 +5178,15 @@ jnewblk_merge(new, old, wkhd) return (new); /* Replace a jfreefrag with a jnewblk. */ if (new->wk_type == D_JFREEFRAG) { + if (WK_JNEWBLK(old)->jn_blkno != WK_JFREEFRAG(new)->fr_blkno) + panic("jnewblk_merge: blkno mismatch: %p, %p", + old, new); cancel_jfreefrag(WK_JFREEFRAG(new)); return (old); } + if (old->wk_type != D_JNEWBLK || new->wk_type != D_JNEWBLK) + panic("jnewblk_merge: Bad type: old %d new %d\n", + old->wk_type, new->wk_type); /* * Handle merging of two jnewblk records that describe * different sets of fragments in the same block. @@ -10507,7 +10513,7 @@ initiate_write_bmsafemap(bmsafemap, bp) ino_t ino; if (bmsafemap->sm_state & IOSTARTED) - panic("initiate_write_bmsafemap: Already started\n"); + return; bmsafemap->sm_state |= IOSTARTED; /* * Clear any inode allocations which are pending journal writes. @@ -10518,10 +10524,6 @@ initiate_write_bmsafemap(bmsafemap, bp) inosused = cg_inosused(cgp); LIST_FOREACH(jaddref, &bmsafemap->sm_jaddrefhd, ja_bmdeps) { ino = jaddref->ja_ino % fs->fs_ipg; - /* - * If this is a background copy the inode may not - * be marked used yet. - */ if (isset(inosused, ino)) { if ((jaddref->ja_mode & IFMT) == IFDIR) cgp->cg_cs.cs_ndir--; @@ -10530,7 +10532,7 @@ initiate_write_bmsafemap(bmsafemap, bp) jaddref->ja_state &= ~ATTACHED; jaddref->ja_state |= UNDONE; stat_jaddref++; - } else if ((bp->b_xflags & BX_BKGRDMARKER) == 0) + } else panic("initiate_write_bmsafemap: inode %d " "marked free", jaddref->ja_ino); } @@ -10545,9 +10547,8 @@ initiate_write_bmsafemap(bmsafemap, bp) LIST_FOREACH(jnewblk, &bmsafemap->sm_jnewblkhd, jn_deps) { if (jnewblk_rollback(jnewblk, fs, cgp, blksfree)) continue; - if ((bp->b_xflags & BX_BKGRDMARKER) == 0) - panic("initiate_write_bmsafemap: block %jd " - "marked free", jnewblk->jn_blkno); + panic("initiate_write_bmsafemap: block %jd " + "marked free", jnewblk->jn_blkno); } } /* @@ -11282,12 +11283,24 @@ diradd_inode_written(dap, inodedep) * only be called with lk and the buf lock on the cg held. */ static int -bmsafemap_rollbacks(bmsafemap) +bmsafemap_backgroundwrite(bmsafemap, bp) struct bmsafemap *bmsafemap; + struct buf *bp; { + int dirty; - return (!LIST_EMPTY(&bmsafemap->sm_jaddrefhd) | - !LIST_EMPTY(&bmsafemap->sm_jnewblkhd)); + dirty = !LIST_EMPTY(&bmsafemap->sm_jaddrefhd) | + !LIST_EMPTY(&bmsafemap->sm_jnewblkhd); + /* + * If we're initiating a background write we need to process the + * rollbacks as they exist now, not as they exist when IO starts. + * No other consumers will look at the contents of the shadowed + * buf so this is safe to do here. + */ + if (bp->b_xflags & BX_BKGRDMARKER) + initiate_write_bmsafemap(bmsafemap, bp); + + return (dirty); } /*