Re: [PATCH 5/9] xfs: make xfs_buf_read_map return an error code

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Jan 16, 2020 at 08:21:48AM -0800, Christoph Hellwig wrote:
> > @@ -2960,6 +2960,10 @@ xfs_read_agf(
> >  			mp, tp, mp->m_ddev_targp,
> >  			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
> >  			XFS_FSS_TO_BB(mp, 1), flags, bpp, &xfs_agf_buf_ops);
> > +	if (error == -EAGAIN && (flags & XBF_TRYLOCK)) {
> 
> Given that EAGAIN is only returned in the XBF_TRYLOCK case the check
> for XBF_TRYLOCK should not be required.

Ok.

> > +		*bpp = NULL;
> > +		return 0;
> 
> Also we should make sure in the lowest layers to never return a
> non-NULL bpp if returning an error to avoid this boilerplate code.

<nod>

At some point I was planning to audit the ALLOC_TRYLOCK cases to make
sure that they can handle an EAGAIN, so we can get rid of this chunk
entirely.

> > -	if (!bp)
> > -		return -ENOMEM;
> > +	error = xfs_buf_read_map(target, &map, 1, flags, &bp, ops);
> > +	if (error)
> > +		return error;
> >  	error = bp->b_error;
> >  	if (error) {
> >  		xfs_buf_ioerror_alert(bp, __func__);
> 
> The extra checking of bp->b_error shoudn't be required now.  That almost
> means we might have to move the xfs_buf_ioerror_alert into
> xfs_buf_read_map.
> 
> That also means xfs_buf_read can be turned into a simple inline
> function again.

Yeah, I'll add another patch to factor that out.

--D