linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] readlink cleanup
@ 2016-12-10 20:41 Miklos Szeredi
  2016-12-11  2:36 ` Al Viro
  0 siblings, 1 reply; 4+ messages in thread
From: Miklos Szeredi @ 2016-12-10 20:41 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, linux-fsdevel, linux-unionfs

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(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [GIT PULL] readlink cleanup
  2016-12-10 20:41 [GIT PULL] readlink cleanup Miklos Szeredi
@ 2016-12-11  2:36 ` Al Viro
  2016-12-11  2:39   ` Al Viro
  0 siblings, 1 reply; 4+ messages in thread
From: Al Viro @ 2016-12-11  2:36 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-kernel, linux-fsdevel, linux-unionfs

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)...

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [GIT PULL] readlink cleanup
  2016-12-11  2:36 ` Al Viro
@ 2016-12-11  2:39   ` Al Viro
  2016-12-11 10:01     ` Miklos Szeredi
  0 siblings, 1 reply; 4+ messages in thread
From: Al Viro @ 2016-12-11  2:39 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-kernel, linux-fsdevel, linux-unionfs

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [GIT PULL] readlink cleanup
  2016-12-11  2:39   ` Al Viro
@ 2016-12-11 10:01     ` Miklos Szeredi
  0 siblings, 0 replies; 4+ messages in thread
From: Miklos Szeredi @ 2016-12-11 10:01 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, linux-fsdevel, linux-unionfs

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-11 10:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-10 20:41 [GIT PULL] readlink cleanup Miklos Szeredi
2016-12-11  2:36 ` Al Viro
2016-12-11  2:39   ` Al Viro
2016-12-11 10:01     ` Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).