HI Christoph, Some clarifications sought.... On Fri, 2011-07-15 at 21:59 -0400, Christoph Hellwig wrote: > > --- a/fs/xfs/xfs_log.c > > +++ b/fs/xfs/xfs_log.c > > @@ -1367,7 +1367,7 @@ xlog_sync(xlog_t *log, > > } > > XFS_BUF_SET_COUNT(bp, count); > > XFS_BUF_SET_FSPRIVATE(bp, iclog); /* save for later */ > > - XFS_BUF_ZEROFLAGS(bp); > > + bp->b_flags &= ~(XBF_READ|XBF_WRITE|XBF_ASYNC|XBF_DELWRI|XBF_ORDERED); > > XFS_BUF_BUSY(bp); > > XFS_BUF_ASYNC(bp); > > bp->b_flags |= XBF_LOG_BUFFER; > > Note that XBF_ORDERED is gone in the current tree, you need to rebase > your patches to pick up the buffer changes I recently sent out. > > I don't link this blind expansion. For one all that flag setting is > duplicated in two places in xlog_sync, please send a patch to > consolidate it in xlog_bdstrat. Second XBF_READ and XBF_DELWRI The two places are using 2 different bps, how do you want it to be consolidated in xlog_bdstrat() ? > won't ever be set here, so no need to clear them. And XBF_WRITE > and XBF_ASYNC will get re-set a few lines down the code, so no need > to clear them either. The XBF_FLUSH and XBF_FUA flag also get set > conditionally a little below, so I'd just clear them in an else > clause for the if condition checking if they should be set. It looks cleaner if I just have those flags cleared above, instead of conditionally clearing them, what do you think ? > > > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > > index 04142ca..75ba081 100644 > > --- a/fs/xfs/xfs_log_recover.c > > +++ b/fs/xfs/xfs_log_recover.c > > @@ -261,7 +261,7 @@ xlog_bwrite( > > ASSERT(BBTOB(nbblks) <= XFS_BUF_SIZE(bp)); > > > > XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no); > > - XFS_BUF_ZEROFLAGS(bp); > > + bp->b_flags &= ~(XBF_READ|XBF_WRITE|XBF_ASYNC|XBF_DELWRI|XBF_ORDERED); > > XFS_BUF_BUSY(bp); > > XFS_BUF_HOLD(bp); > > XFS_BUF_PSEMA(bp, PRIBIO); > > XBF_READ, XBF_WRITE, XBF_ASYNC and XBF_DELWRI already get dealt with > in xfs_bwrite a few lines down, and XBF_ORDERED can't be set here. IOW > this line can simply be removed. New definition of XFS_BUF_ZEROFLAGS has these: XBF_SYNCIO, XBF_FUA, and XBF_FLUSH. In this context is it ok to not clear those ? > _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
