Re: [PATCH 03/24] xfs: mark inode buffers in cache

Dave Chinner
2020-05-24T23:41:46Z

On Fri, May 22, 2020 at 02:35:17PM -0700, Darrick J. Wong wrote:
> On Fri, May 22, 2020 at 01:50:08PM +1000, Dave Chinner wrote:
> > #define XFS_BUF_FLAGS \
> > @@ -50,12 +54,13 @@ typedef unsigned int xfs_buf_flags_t;
> > { XBF_DONE, "DONE" }, \
> > { XBF_STALE, "STALE" }, \
> > { XBF_WRITE_FAIL, "WRITE_FAIL" }, \
> > - { XBF_TRYLOCK, "TRYLOCK" }, /* should never be set */\
> > - { XBF_UNMAPPED, "UNMAPPED" }, /* ditto */\
> > + { _XBF_INODES, "INODES" }, \
>
> This a toughie. On the one hand if you're going to go introducing what
> amounts to two-bit buffer io completion type in the middle of b_flags
> then (like Amir says) this ideally would have a mask and switch
> statements and whatnot.

I didn't really want a mask, because these are not unconditional
"buffer type" flags. They are only set on buffers with inodes
attached for IO completion and used that way in this patchset.
Everywhere else that uses them only checks for a single type...

> I also wonder if we could tell the buffer type given all the
> xfs_trans_buf_set_type calls, but I think the answer is that not every
> buffer is guaranteed to have a buffer log item attached and a type code
> set correctly?

Correct - that's what I looked at first, but inode cluster buffers
that have not just been created or have an unlinked inode logged
against them don't have buffer log items. Similarly dquot buffers
may not have a log item, either.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com