Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1539907 > unrolled thread
| Started by | Miklos Szeredi <miklos@szeredi.hu> |
|---|---|
| First post | 2016-12-10 21:50 +0100 |
| Last post | 2016-12-11 11:10 +0100 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
[GIT PULL] readlink cleanup Miklos Szeredi <miklos@szeredi.hu> - 2016-12-10 21:50 +0100
Re: [GIT PULL] readlink cleanup Al Viro <viro@ZenIV.linux.org.uk> - 2016-12-11 03:40 +0100
Re: [GIT PULL] readlink cleanup Al Viro <viro@ZenIV.linux.org.uk> - 2016-12-11 03:40 +0100
Re: [GIT PULL] readlink cleanup Miklos Szeredi <miklos@szeredi.hu> - 2016-12-11 11:10 +0100
| From | Miklos Szeredi <miklos@szeredi.hu> |
|---|---|
| Date | 2016-12-10 21:50 +0100 |
| Subject | [GIT PULL] readlink cleanup |
| Message-ID | <sMWKK-6RV-11@gated-at.bofh.it> |
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(-)
[toc] | [next] | [standalone]
| From | Al Viro <viro@ZenIV.linux.org.uk> |
|---|---|
| Date | 2016-12-11 03:40 +0100 |
| Message-ID | <sN2dr-1Js-3@gated-at.bofh.it> |
| In reply to | #1539907 |
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)...
[toc] | [prev] | [next] | [standalone]
| From | Al Viro <viro@ZenIV.linux.org.uk> |
|---|---|
| Date | 2016-12-11 03:40 +0100 |
| Message-ID | <sN2dr-1Js-7@gated-at.bofh.it> |
| In reply to | #1539954 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Miklos Szeredi <miklos@szeredi.hu> |
|---|---|
| Date | 2016-12-11 11:10 +0100 |
| Message-ID | <sN9eV-6kb-1@gated-at.bofh.it> |
| In reply to | #1539955 |
On Sun, Dec 11, 2016 at 3:39 AM, Al Viro <viro@zeniv.linux.org.uk> 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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web