Re: [PATCH 2/5] xfs: replace xfs_buf_incore with an XBF_NOALLOC flag to xfs_buf_get*

[Dave Chinner <david@fromorbit.com>]

On Tue, Apr 05, 2022 at 04:55:09PM +0200, Christoph Hellwig wrote:
> On Mon, Apr 04, 2022 at 07:54:43AM +1000, Dave Chinner wrote:
> > /*
> >  * Lock and return the buffer that matches the requested range if
> >  * and only if it is present in the cache already.
> >  */
> > static inline struct xfs_buf *
> > xfs_buf_incore(
> > 	struct xfs_buftarg	*target,
> > 	xfs_daddr_t		blkno,
> > 	size_t			numblks,
> > 	xfs_buf_flags_t		flags)
> > {
> > 	struct xfs_buf		*bp;
> > 	int			error;
> > 	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
> > 
> > 	error = xfs_buf_get_map(target, &map, 1, _XBF_NOALLOC | flags,
> > 				NULL, &bp);
> > 	if (error)
> > 		return NULL;
> > 	return bp;
> > }
> > 
> > Then none of the external callers need to be changed, and we don't
> > introduce new external xfs_buf_get() callers.
> 
> I had that earlier, but having xfs_buf_incore as the odd one out that
> still returns a buffer (like most XFS buffer cache routines did back
> a long time ago) just did seem pretty odd compared tothe rest.

Then let's fix that to use the same interface as everything else,
and that simplifies the implementation down to just:

static inline int
xfs_buf_incore(
	struct xfs_buftarg	*target,
	xfs_daddr_t		blkno,
	size_t			numblks,
	xfs_buf_flags_t		flags,
	struct xfs_buf		**bpp)
{
	DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);

	return xfs_buf_get_map(target, &map, 1, _XBF_INCORE | flags,
				NULL, bpp);
}

And, FWIW, the _XBF_NOALLOC flag really wants to be _XBF_INCORE - we
need it to describe the lookup behaviour the flag provides, not the
internal implementation detail that acheives the desired
behaviour....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com