Re: [PATCH 2/3] xfs: Rename __xfs_buf_submit to xfs_buf_submit

[Dave Chinner <david@fromorbit.com>]

On Tue, Aug 13, 2019 at 07:56:58AM -0400, Brian Foster wrote:
> On Tue, Aug 13, 2019 at 12:03:05PM +0300, Nikolay Borisov wrote:
> > Since xfs_buf_submit no longer has any callers just rename its __
> > prefixed counterpart.
> > 
> > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> > ---
> 
> Now we have a primary submission interface that allows combinations of
> XBF_ASYNC and waiting or not while the underlying mechanisms are not so
> flexible. It looks like the current factoring exists to support delwri
> queues where we never wait in buffer submission regardless of async
> state because we are batching the submission/wait across multiple
> buffers. But what happens if a caller passes an async buffer with wait
> == true? I/O completion only completes ->b_iowait if XBF_ASYNC is clear.
> 
> I find this rather confusing because now a caller needs to know about
> implementation details to use the function properly. That's already true
> of __xfs_buf_submit(), but that's partly why it's named as an "internal"
> function. I think we ultimately need the interface flexibility so the
> delwri case can continue to work. One option could be to update
> xfs_buf_submit() such that we never wait on an XBF_ASYNC buffer and add
> an assert to flag wait == true as invalid, but TBH I'm not convinced
> this is any simpler than the current interface where most callers simply
> only need to care about the flag. Maybe others have thoughts...

Yeah, we slpit the code u plike this intentionally to separate out
the different ways of submitting IO so that we didn't end up using
invalid methods, like ASYNC + wait, which would lead to hangs
waiting for IO that has already completed.

I much prefer the code as it stands now - it may be slightly more
verbose, but it's simple to understand and hard to use
incorrectly....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com