Hi Al,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git readlink
This is a rework of the readlink cleanup patchset. Now readlink(2) does the
following:
- if i_op->readlink() is non-NULL (only proc and afs mountpoints for now) then
it calls that;
- otherwise call i_op->get_link();
- signature of ->readlink() now matches that of ->get_link();
This is against the #misc branch (the stats look better that way ;)
Thanks,
Miklos
---
Miklos Szeredi (10):
ecryptfs: use vfs_get_link()
proc/self: 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 | 8 +---
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 | 58 ++++++++++++++++-----------
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/inode.c | 1 -
fs/proc/base.c | 57 +++++++++-----------------
fs/proc/inode.c | 1 -
fs/proc/namespaces.c | 13 +++---
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 | 9 ++---
include/linux/proc_ns.h | 4 +-
mm/shmem.c | 2 -
52 files changed, 124 insertions(+), 195 deletions(-)
On Sat, Dec 10, 2016 at 09:41:45PM +0100, Miklos Szeredi wrote:
> Hi Al,
>
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git readlink
>
> This is a rework of the readlink cleanup patchset. Now readlink(2) does the
> following:
>
> - if i_op->readlink() is non-NULL (only proc and afs mountpoints for now) then
> it calls that;
> - otherwise call i_op->get_link();
> - signature of ->readlink() now matches that of ->get_link();
>
> This is against the #misc branch (the stats look better that way ;)
I'm still not sure what does "vfs: convert ->readlink to same signature as
->get_link" buy us. If anything, the result appears to be more complex -
you make freeing that buffer delayed (and introduce a dynamically allocated
buffer in one case that didn't use it)...
On Sun, Dec 11, 2016 at 02:36:20AM +0000, Al Viro wrote:
> I'm still not sure what does "vfs: convert ->readlink to same signature as
> ->get_link" buy us. If anything, the result appears to be more complex -
> you make freeing that buffer delayed (and introduce a dynamically allocated
> buffer in one case that didn't use it)...
PS: everything prior to that point looks fine.
On Sun, Dec 11, 2016 at 3:39 AM, Al Viro <[email protected]> wrote:
> On Sun, Dec 11, 2016 at 02:36:20AM +0000, Al Viro wrote:
>
>> I'm still not sure what does "vfs: convert ->readlink to same signature as
>> ->get_link" buy us. If anything, the result appears to be more complex -
>> you make freeing that buffer delayed (and introduce a dynamically allocated
>> buffer in one case that didn't use it)...
Normally readlink(2) calls ->get_link() except if there's
->readlink(). So there's no added complexity in handling the delayed
free since it's already there. In fact it allows for removal of
complexity.
But I think the diffstat of that last part speaks for itself:
11 files changed, 76 insertions(+), 110 deletions(-)
Btw, in the one case where we added the dynamically allocated buffer
it had been:
- guess max link size to be 50 (very scientifically I'm sure, but no
explanation given)
- call filler
- hope it didn't get truncated
Which is now
- call filler to allocate correctly sized buffer
Which isn't even much more complex. So I don't buy your arguments.
Thanks,
Miklos