On Thu, 2021-12-16 at 16:11 +1100, Dave Chinner wrote: > On Wed, Dec 15, 2021 at 05:09:48PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > > > Ian Kent reported that for inline symlinks, it's possible for > > vfs_readlink to hang on to the target buffer returned by > > _vn_get_link_inline long after it's been freed by xfs inode > > reclaim. > > This is a layering violation -- we should never expose XFS > > internals to > > the VFS. > > > > When the symlink has a remote target, we allocate a separate > > buffer, > > copy the internal information, and let the VFS manage the new > > buffer's > > lifetime. Let's adapt the inline code paths to do this too. It's > > less efficient, but fixes the layering violation and avoids the > > need to > > adapt the if_data lifetime to rcu rules. Clearly I don't care > > about > > readlink benchmarks. > > > > As a side note, this fixes the minor locking violation where we can > > access the inode data fork without taking any locks; proper locking > > (and > > eliminating the possibility of having to switch inode_operations on > > a > > live inode) is essential to online repair coordinating repairs > > correctly. > > > > Reported-by: Ian Kent <raven@xxxxxxxxxx> > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> > > Looks fine, nicely avoids all the nasty RCU interactions trying to > handle this after the fact. > > Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx> > Yes, I like it too and that original rcu gymnastics was always due to the unclear ownership and lifetime of the link path buffer. And I don't think needing to switch to ref-walk mode (due to the memory allocation possibly blocking) is such a big performance drawback as might be thought. Acked-by: Ian Kent <raven@xxxxxxxxxx>
