Re: [PATCH 5/6] xfs: move non-inline symlinks to the pagecache

Re: [PATCH 5/6] xfs: move non-inline symlinks to the pagecache

[Christoph Hellwig]

Al, what do you think about adding a new

i_link member to the union of i_pipe, i_bdev and i_cdev.  That we
we can cache a link acquired by any way for direct use in the VFS.

This has a few use cases:  inline links can be set up directly
when reading the inode, and we never need to call into ->follow_link.

Formats like the XFS v5 symlinks can be read in once by whatever
way we want, and following accesses can be done RCU safe and
without calling into the filesystem.

Note that caching the symlink in a kmalloc'ed buffer might be
more efficient than the pagecache for most cases anyway.

Re: [PATCH 5/6] xfs: move non-inline symlinks to the pagecache

[Al Viro]

On Sat, Apr 25, 2015 at 04:16:12PM +0200, Christoph Hellwig wrote:
> Al, what do you think about adding a new
> 
> i_link member to the union of i_pipe, i_bdev and i_cdev.  That we
> we can cache a link acquired by any way for direct use in the VFS.
> 
> This has a few use cases:  inline links can be set up directly
> when reading the inode, and we never need to call into ->follow_link.
> 
> Formats like the XFS v5 symlinks can be read in once by whatever
> way we want, and following accesses can be done RCU safe and
> without calling into the filesystem.
> 
> Note that caching the symlink in a kmalloc'ed buffer might be
> more efficient than the pagecache for most cases anyway.

Hmm...  When would you free the sucker?

Re: [PATCH 5/6] xfs: move non-inline symlinks to the pagecache

[Al Viro]

On Sat, Apr 25, 2015 at 03:57:28PM +0100, Al Viro wrote:
> > i_link member to the union of i_pipe, i_bdev and i_cdev.  That we
> > we can cache a link acquired by any way for direct use in the VFS.
> > 
> > This has a few use cases:  inline links can be set up directly
> > when reading the inode, and we never need to call into ->follow_link.
> > 
> > Formats like the XFS v5 symlinks can be read in once by whatever
> > way we want, and following accesses can be done RCU safe and
> > without calling into the filesystem.
> > 
> > Note that caching the symlink in a kmalloc'ed buffer might be
> > more efficient than the pagecache for most cases anyway.
> 
> Hmm...  When would you free the sucker?

FWIW, I'm not particularly opposed to doing that, but we'd better be careful
about not losing ->follow_link() itself.  Reason: we use its presence to
tell symlinks from non-symlinks.  OTOH, something like

	/* have already decided it's a symlink */
	if (inode->i_link)
		return inode->i_link;
	res = inode->i_op->follow_link(...);
with ->follow_link() instance returning ERR_PTR(-EIO) would work.  Such
sucker could live in fs/libfs.c just fine, with rule being "if you use it
for ->follow_link(), you'd better set ->i_link"...

Note, BTW, that there are symlinks where we _do_ have "traverse a string"
for semantics, and it's even kmalloc'ed, but we very much do not want it
to be cached.  Consider /proc/self, for example.  Different processes should
see different link bodies there, without any serialization between them.

Re: [PATCH 5/6] xfs: move non-inline symlinks to the pagecache

[Christoph Hellwig]

On Sat, Apr 25, 2015 at 03:57:28PM +0100, Al Viro wrote:
> > Note that caching the symlink in a kmalloc'ed buffer might be
> > more efficient than the pagecache for most cases anyway.
> 
> Hmm...  When would you free the sucker?

final iput.  Similar design to the generic ACL cache.

Re: [PATCH 5/6] xfs: move non-inline symlinks to the pagecache

[Al Viro]

On Sat, Apr 25, 2015 at 08:32:47PM +0200, Christoph Hellwig wrote:
> On Sat, Apr 25, 2015 at 03:57:28PM +0100, Al Viro wrote:
> > > Note that caching the symlink in a kmalloc'ed buffer might be
> > > more efficient than the pagecache for most cases anyway.
> > 
> > Hmm...  When would you free the sucker?
> 
> final iput.  Similar design to the generic ACL cache.

Except that in this case you have to deal with the cases when it should
_not_ be freed in ->evict_inode() (and doing that in generic code is
right out).  I'm not sure it will be simpler that way, actually...