Re: [PATCH] xfs: open code end_buffer_async_write in xfs_finish_page_writeback

[Christoph Hellwig <hch@lst.de>]

On Fri, Aug 11, 2017 at 09:20:38AM -0700, Darrick J. Wong wrote:
> If we're going to open-code end_buffer_async_write, could we have a
> comment here explaining that we've done so, and why?

Sure.

> ASSERT(bh->b_end_io == end_buffer_async_write), just in case we ever get
> passed a buffer head with an endio function that isn't what we're
> open-coding?

Ok.

> > +	if (unlikely(error && !quiet)) {
> > +		xfs_err_ratelimited(XFS_I(inode)->i_mount,
> > +			"read error on sector %llu", start);
> 
> Read error?  Isn't this writeback?

Ooops, it is.

> I also wonder about the wisdom of the text deviating from "lost async
> page write", since scripts/admins have been trained to look for that as
> evidence of writeback problems for years.

We could keep the text, but I think it's rather confusing and doesn't
fit the other XFS warnings.  And I don't think admins should rely on
the sorts of warnings.  E.g. for btrfs they would not get these
warnings either.

> (That said, at least now it's a lot less spammy.)

Yes - but we could still print the warning once per bio instead of
once per bh if the message remains the same.