Re: [PATCH] xfs: use inode_set_cached_link()

[Mateusz Guzik]

On Sat, Nov 23, 2024 at 5:19 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Sat, Nov 23, 2024 at 08:51:05AM +0100, Mateusz Guzik wrote:
> > For cases where caching is applicable this dodges inode locking, memory
> > allocation and memcpy + strlen.
> >
> > Throughput of readlink on Saphire Rappids (ops/s):
> > before:       3641273
> > after:        4009524 (+10%)
> >
> > Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>
> > ---
> >
> > First a minor note that in the stock case strlen is called on the buffer
> > and I verified that i_disk_size is the value which is computed.
> >
> > The important note is that I'm assuming the pointed to area is stable
> > for the duration of the inode's lifetime -- that is if the read off
> > symlink is fine *or* it was just created and is eligible caching, it
> > wont get invalidated as long as the inode is in memory. If this does not
> > hold then this submission is wrong and it would be nice(tm) to remedy
> > it.
>
> It is not stable for the lifetime of the inode.  See commit
> 7b7820b83f2300 ("xfs: don't expose internal symlink metadata buffers to
> the vfs").  With parent pointers' ability to expand the symlink xattr
> fork area sufficiently to bump the symlink target into a remote block
> and online repair's ability to mess with the inode, direct vfs access of
> if_data has only become more difficult.
>

That's a bummer.

Thanks for the review.
-- 
Mateusz Guzik <mjguzik gmail.com>