linux-kernel - Re: [PATCH] xfs: use inode_set_cached

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGudoHEHSoFzadeNW3dhStCVy03G61X+Rd50SQKYt=aDRv6BYw@mail.gmail.com>
Date: Sat, 23 Nov 2024 19:52:50 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: dchinner@...hat.com, cem@...nel.org, linux-kernel@...r.kernel.org, 
	linux-xfs@...r.kernel.org
Subject: Re: [PATCH] xfs: use inode_set_cached_link()

On Sat, Nov 23, 2024 at 5:19 PM Darrick J. Wong <djwong@...nel.org> 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@...il.com>
> > ---
> >
> > 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>