On Wed, Apr 06, 2022 at 07:21:33AM +1000, Dave Chinner wrote:
> > 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....
At least in my mental model a 'find but do not allocate' matches
the lookup behavior more than the somewhat odd 'incore' name. I know
it is something traditional Unix including IRIX has used forever,
but it is a bit of an odd choice with no history in Linux.
That being said the flag and the wrapper should match, so IFF we keep
xfs_buf_incore the flag should also be _XBF_INCORE. Still not my
preference, though.
