On Wed, Apr 10, 2019 at 04:44:14AM +0100, Al Viro wrote:
> On Tue, Apr 09, 2019 at 07:58:08PM -0700, Eric Biggers wrote:
>
> > It could check a flag IOP_GET_LINK in ->i_opflags instead, so it would be the
> > same number of checks. See patch below.
>
> With that patch ->i_link is completely unused if ->get_link() is non-NULL,
> so you get a method call on each traversal...
>
.get_link would be left NULL in all inode_operations that currently use
simple_get_link, then simple_get_link() would be removed. My example patch just
changed it in ext4 as an example.
> > Benefits are that we get code that isn't actively misleading (via
> > simple_get_link() existing but actually never being called), and filesystems can
> > cache a symlink target in ->i_link if it becomes available later, i.e. if it's
> > not immediately available at iget() time. Otherwise a filesystem-private field
> > has to be used instead. (For fscrypt, I'd probably use fscrypt_info::ci_link.)
>
> What's to stop you from doing just that right now? You'd need to take
> care with barriers, but you'd need that anyway... As soon as ->i_link is set
> you'll get no more ->get_link() on that sucker, using the cached value
> from that point on. IDGI...
1.) The VFS won't know to drop of RCU-walk mode, so waiting an RCU grace period
before freeing the symlink target becomes mandatory. (Which I'd like to do
for fscrypt anyway, but doing it sanely appears to require implementing
.destroy_inode() for ext4, f2fs, and ubifs. I hoped I could do non-RCU mode
as a simpler first step.)
2.) The VFS won't know to use a read memory barrier when loading i_link.
The VFS could issue one unconditionally, but it would be unnecessary for
regular fast symlinks.
- Eric
