Re: [PATCH 3/4] xfs: remove most in-flight buffer accounting

[Christoph Hellwig <hch@lst.de>]

On Tue, Feb 18, 2025 at 12:23:27PM -0800, Darrick J. Wong wrote:
> On Mon, Feb 17, 2025 at 10:31:28AM +0100, Christoph Hellwig wrote:
> > The buffer cache keeps a bt_io_count per-CPU counter to track all
> > in-flight I/O, which is used to ensure no I/O is in flight when
> > unmounting the file system.
> > 
> > For most I/O we already keep track of inflight I/O at higher levels:
> > 
> >  - for synchronous I/O (xfs_buf_read/xfs_bwrite/xfs_buf_delwri_submit),
> >    the caller has a reference and waits for I/O completions using
> >    xfs_buf_iowait
> >  - for xfs_buf_delwri_submit the only caller (AIL writeback) tracks the
> 
> Do you mean xfs_buf_delwri_submit_nowait here?

Yes.

> IOWs, only asynchronous readahead needs an explicit counter in the
> xfs_buf to prevent unmount because:
> 
> 0. Anything done in mount/unmount/freeze holds s_umount
> 1. Buffer reads done on behalf of a file hold the file open and pin the
>    mount
> 2. Dirty buffers have log items, and we can't unmount until those are
>    dealt with
> 3. Fsck holds an open fd and hence pins the mount
> 4. Unmount blocks until background gc finishes
> 
> Right?

Yes.

> I almost wonder if you could just have a percpu counter in the
> xfs_mount but that sounds pretty hot.

Well, that would remove the nice xfs_buftarg_wait() abstraction.
Givne that we don't even allocate an extra buftrag unless we use
it that doesn't seem very useful.

> > +	/* there are currently no valid flags for xfs_buf_get_uncached */
> > +	ASSERT(flags == 0);
> 
> Can we just get rid of flags then?  AFAICT nobody uses it either here or
> in xfsprogs, and in fact I think there's a nasty bug in the userspace
> rtsb code:

See my reply to the last patch: I actually have a patch to remove it,
but it conflicts with the zoned series.  So for now I'll defer it until
that is merged.