On Sat, May 23, 2020 at 01:48:27AM -0700, Christoph Hellwig wrote: > On Fri, May 22, 2020 at 01:50:08PM +1000, Dave Chinner wrote: > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > Inode buffers always have write IO callbacks, so by marking them > > directly we can avoid needing to attach ->b_iodone functions to > > them. This avoids an indirect call, and makes future modifications > > much simpler. > > > > This is largely a rearrangement of the code at this point - no IO > > completion functionality changes at this point, just how the > > code is run is modified. > > So I've looked over the whole series where this leads, and I really like > killing off li_cb and sorting out the mess around b_iodone. But I think > the series ends up moving too much out of xfs_buf.c. I'd rather keep > a minimal b_write_done callback rather than the checking of the inode > and dquote flags, and keep most of the logic in xfs_buf.c. This is the > patch I cooked up on top of your series to show what I mean - it removes > the need for both flags and kills about 100 lines of code: Let's deal with further cleanups after this patchset and the follow-on patchsets that I have are sorted out. All cleanups will do right now is invalidate all the stuff I'm still working on that sits on top of this patchset.. > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c > index e5f58a4b13eee..4220fee1c9ddb 100644 > --- a/fs/xfs/libxfs/xfs_trans_inode.c > +++ b/fs/xfs/libxfs/xfs_trans_inode.c > @@ -169,7 +169,7 @@ xfs_trans_log_inode( > xfs_buf_hold(bp); > spin_lock(&iip->ili_lock); > iip->ili_item.li_buf = bp; > - bp->b_flags |= _XBF_INODES; > + bp->b_write_done = xfs_iflush_done; I originally tried this, but then I ended up with places in follow-on patchsets where I had to do this sort of check: if (bp->b_write_done == xfs_iflush_done) To detect a buffer that already had inodes attached to it. Which clearly told me that there should be a state flag to indicate that the buffer has attached inodes.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
