On Wed, Sep 28, 2016 at 4:17 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> Symlink traversal is a much hotter path than readlink() would ever be.
> What's more, we do have jumps on normal symlink traversal - after all,
> absolute symlinks are exactly that; it's "jump to root, then traverse
> the following sequence of components". So having ->get_link() that
> includes jumps is not that much of a stretch (note that it could both
> jump and return a relative pathname to traverse after that; none of the
> procfs-style ones do that, but there's no reason to prohibit that).
>
> What I'd prefer is
> * it's a symlink iff it has ->get_link()
> * readlink(2) on a symlink is normally just using generic_readlink()
> * that can be overridden by supplying a ->readlink() method.
> * the first time readlink() hits a symlink it will check both
> ->get_link() and ->readlink() presence. Then, if it's a normal symlink,
> the inode will get marked as such and all subsequent calls will just call
> generic_readlink(). IOW, I would go for
> if (unlikely(!marked)) {
> if ->readlink is present
> call ->readlink and return
> if ->get_link is absent
> fail
> mark
> }
> call generic_readlink
Redid patchset confirming to the above.
Also moved the overlayfs/ecryptfs fixes to the front of the queue, as
they are independent of the rest of the cleanups.
The last part is new: changing ->readlink signature to match that of
->get_link. This moves the user buffer handling out of filesystems,
simplifying the implementation in the process.
Force pushed new set to:
git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git #readlink
It's got a couple of trivial conflicts against #rename2 and
#overlayfs-next. I can do the merge and send a big pull request if
you want.
Thanks,
Miklos
---
Miklos Szeredi (14):
vfs: add vfs_get_link() helper
ovl: use vfs_get_link()
ecryptfs: use vfs_get_link()
ovl: use generic_readlink
proc/self: use generic_readlink
bad_inode: use generic_readlink
vfs: replace calling i_op->readlink with vfs_readlink()
vfs: default to generic_readlink()
vfs: remove ".readlink = generic_readlink" assignments
vfs: make generic_readlink() static
vfs: convert ->readlink to same signature as ->get_link
vfs: remove page_readlink()
vfs: make readlink_copy() static
nsfs: clean up ns_get_name() interface
---
Documentation/filesystems/Locking | 6 +-
Documentation/filesystems/porting | 7 +++
Documentation/filesystems/vfs.txt | 14 +++--
drivers/staging/lustre/lustre/llite/symlink.c | 1 -
fs/9p/vfs_inode.c | 1 -
fs/9p/vfs_inode_dotl.c | 1 -
fs/affs/symlink.c | 1 -
fs/afs/mntpt.c | 2 +-
fs/autofs4/symlink.c | 1 -
fs/bad_inode.c | 7 ---
fs/btrfs/inode.c | 1 -
fs/ceph/inode.c | 1 -
fs/cifs/cifsfs.c | 1 -
fs/coda/cnode.c | 1 -
fs/configfs/symlink.c | 1 -
fs/ecryptfs/inode.c | 30 ++++------
fs/ext2/symlink.c | 2 -
fs/ext4/symlink.c | 3 -
fs/f2fs/namei.c | 2 -
fs/fuse/dir.c | 1 -
fs/gfs2/inode.c | 1 -
fs/hostfs/hostfs_kern.c | 1 -
fs/jffs2/symlink.c | 1 -
fs/jfs/symlink.c | 2 -
fs/kernfs/symlink.c | 1 -
fs/libfs.c | 1 -
fs/minix/inode.c | 1 -
fs/namei.c | 83 +++++++++++++++++++--------
fs/ncpfs/inode.c | 1 -
fs/nfs/symlink.c | 1 -
fs/nfsd/nfs4xdr.c | 8 +--
fs/nfsd/vfs.c | 6 +-
fs/nilfs2/namei.c | 1 -
fs/nsfs.c | 17 ++++--
fs/ocfs2/symlink.c | 1 -
fs/orangefs/symlink.c | 1 -
fs/overlayfs/copy_up.c | 46 ++-------------
fs/overlayfs/inode.c | 30 +---------
fs/proc/base.c | 57 +++++++-----------
fs/proc/inode.c | 1 -
fs/proc/namespaces.c | 15 +++--
fs/proc/self.c | 13 -----
fs/proc/thread_self.c | 14 -----
fs/reiserfs/namei.c | 1 -
fs/squashfs/symlink.c | 1 -
fs/stat.c | 8 ++-
fs/sysv/inode.c | 1 -
fs/ubifs/file.c | 1 -
fs/xfs/xfs_ioctl.c | 4 +-
fs/xfs/xfs_iops.c | 2 -
include/linux/fs.h | 10 ++--
include/linux/proc_ns.h | 4 +-
mm/shmem.c | 2 -
53 files changed, 157 insertions(+), 264 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
