* [git pull] vfs.git pile 2
@ 2014-12-15 23:51 Al Viro
2014-12-16 13:33 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2014-12-15 23:51 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel
Next pile (and there'll be one or two more). The large piece in this one is
getting rid of /proc/*/ns/* weirdness; among other things, it allows to
(finally) make nameidata completely opaque outside of fs/namei.c, making for
easier further cleanups in there.
I have _not_ included removal of unshare_fs_struct() into that pile; I think
it'll turn out to be OK, but it's clearly the next cycle fodder.
Please, pull from the usual place -
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus
Shortlog:
Al Viro (20):
common object embedded into various struct ....ns
make mntns ->get()/->put()/->install()/->inum() work with &mnt_ns->ns
netns: switch ->get()/->put()/->install()/->inum() to working with &net->ns
switch the rest of proc_ns_operations to working with &...->ns
make proc_ns_operations work with struct ns_common * instead of void *
new helpers: ns_alloc_inum/ns_free_inum
copy address of proc_ns_ops into ns_common
bury struct proc_ns in fs/proc
take the targets of /proc/*/ns/* symlinks to separate fs
kill proc_ns completely
make nameidata completely opaque outside of fs/namei.c
make default ->i_fop have ->open() fail with ENXIO
path_init(): store the "base" pointer to file in nameidata itself
fs/namei.c: new helper (path_cleanup())
path_init(): don't bother with LOOKUP_PARENT in argument
fs/namei.c: fold link_path_walk() call into path_init()
coda_venus_readdir(): use file_inode()
spawn PID 1 without CLONE_FS, give kernel threads zero umask
lustre: get rid of playing with ->fs
btrfs: filp_open() returns ERR_PTR() on failure, not NULL...
Diffstat:
arch/ia64/kernel/perfmon.c | 10 --
.../lustre/lustre/include/linux/lustre_compat25.h | 24 ---
drivers/staging/lustre/lustre/llite/dir.c | 2 +-
drivers/staging/lustre/lustre/llite/llite_lib.c | 17 +--
fs/Makefile | 2 +-
fs/btrfs/volumes.c | 2 +-
fs/coda/dir.c | 4 +-
fs/fs_struct.c | 2 +-
fs/inode.c | 11 +-
fs/internal.h | 5 +
fs/mount.h | 3 +-
fs/namei.c | 98 +++++++------
fs/namespace.c | 51 +++----
fs/nsfs.c | 161 +++++++++++++++++++++
fs/proc/inode.c | 10 +-
fs/proc/internal.h | 2 +-
fs/proc/namespaces.c | 153 ++------------------
include/linux/fs.h | 1 -
include/linux/ipc_namespace.h | 3 +-
include/linux/namei.h | 25 +---
include/linux/ns_common.h | 12 ++
include/linux/pid_namespace.h | 3 +-
include/linux/proc_ns.h | 43 +++---
include/linux/user_namespace.h | 3 +-
include/linux/utsname.h | 3 +-
include/net/net_namespace.h | 3 +-
include/uapi/linux/magic.h | 1 +
init/main.c | 6 +-
init/version.c | 5 +-
ipc/msgutil.c | 5 +-
ipc/namespace.c | 32 ++--
kernel/kmod.c | 2 +
kernel/nsproxy.c | 10 +-
kernel/pid.c | 5 +-
kernel/pid_namespace.c | 29 ++--
kernel/user.c | 5 +-
kernel/user_namespace.c | 29 ++--
kernel/utsname.c | 31 ++--
net/Makefile | 2 -
net/core/net_namespace.c | 39 ++---
net/nonet.c | 26 ----
net/socket.c | 19 ---
42 files changed, 425 insertions(+), 474 deletions(-)
create mode 100644 fs/nsfs.c
create mode 100644 include/linux/ns_common.h
delete mode 100644 net/nonet.c
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [git pull] vfs.git pile 2
2014-12-15 23:51 [git pull] vfs.git pile 2 Al Viro
@ 2014-12-16 13:33 ` Al Viro
2014-12-16 18:06 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2014-12-16 13:33 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel
On Mon, Dec 15, 2014 at 11:51:25PM +0000, Al Viro wrote:
> Next pile (and there'll be one or two more). The large piece in this one is
> getting rid of /proc/*/ns/* weirdness; among other things, it allows to
> (finally) make nameidata completely opaque outside of fs/namei.c, making for
> easier further cleanups in there.
>
> I have _not_ included removal of unshare_fs_struct() into that pile; I think
> it'll turn out to be OK, but it's clearly the next cycle fodder.
... but I have included the "obviously safe" patch separating PID 1
->fs. Without letting it sit in -next long enough. And it turns out
to be not safe at all - it might be salvagable, but not in this form.
Mea maxima culpa - I should've known better than that. Self-LART applied;
please, drop this pull request, I'll send a saner one (for stuff that *did*
sit in -next long enough) in a bit.
Again, my apologies ;-/
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [git pull] vfs.git pile 2
2014-12-16 13:33 ` Al Viro
@ 2014-12-16 18:06 ` Al Viro
0 siblings, 0 replies; 12+ messages in thread
From: Al Viro @ 2014-12-16 18:06 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel
On Tue, Dec 16, 2014 at 01:33:30PM +0000, Al Viro wrote:
> On Mon, Dec 15, 2014 at 11:51:25PM +0000, Al Viro wrote:
> > Next pile (and there'll be one or two more). The large piece in this one is
> > getting rid of /proc/*/ns/* weirdness; among other things, it allows to
> > (finally) make nameidata completely opaque outside of fs/namei.c, making for
> > easier further cleanups in there.
> >
> > I have _not_ included removal of unshare_fs_struct() into that pile; I think
> > it'll turn out to be OK, but it's clearly the next cycle fodder.
>
> ... but I have included the "obviously safe" patch separating PID 1
> ->fs. Without letting it sit in -next long enough. And it turns out
> to be not safe at all - it might be salvagable, but not in this form.
>
> Mea maxima culpa - I should've known better than that. Self-LART applied;
> please, drop this pull request, I'll send a saner one (for stuff that *did*
> sit in -next long enough) in a bit.
>
> Again, my apologies ;-/
... and this time around - only the stuff that has sat in -next for a while,
with no "obviously safe" additions thrown in. Please, pull from
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus
Shortlog:
Al Viro (17):
common object embedded into various struct ....ns
make mntns ->get()/->put()/->install()/->inum() work with &mnt_ns->ns
netns: switch ->get()/->put()/->install()/->inum() to working with &net->ns
switch the rest of proc_ns_operations to working with &...->ns
make proc_ns_operations work with struct ns_common * instead of void *
new helpers: ns_alloc_inum/ns_free_inum
copy address of proc_ns_ops into ns_common
bury struct proc_ns in fs/proc
take the targets of /proc/*/ns/* symlinks to separate fs
kill proc_ns completely
make nameidata completely opaque outside of fs/namei.c
make default ->i_fop have ->open() fail with ENXIO
path_init(): store the "base" pointer to file in nameidata itself
fs/namei.c: new helper (path_cleanup())
path_init(): don't bother with LOOKUP_PARENT in argument
fs/namei.c: fold link_path_walk() call into path_init()
coda_venus_readdir(): use file_inode()
Diffstat:
arch/ia64/kernel/perfmon.c | 10 ---
fs/Makefile | 2 +-
fs/coda/dir.c | 4 +-
fs/inode.c | 11 ++-
fs/internal.h | 5 ++
fs/mount.h | 3 +-
fs/namei.c | 98 +++++++++++++------------
fs/namespace.c | 51 ++++++-------
fs/nsfs.c | 161 +++++++++++++++++++++++++++++++++++++++++
fs/proc/inode.c | 10 +--
fs/proc/internal.h | 2 +-
fs/proc/namespaces.c | 153 ++++-----------------------------------
include/linux/fs.h | 1 -
include/linux/ipc_namespace.h | 3 +-
include/linux/namei.h | 25 +------
include/linux/ns_common.h | 12 +++
include/linux/pid_namespace.h | 3 +-
include/linux/proc_ns.h | 43 ++++++-----
include/linux/user_namespace.h | 3 +-
include/linux/utsname.h | 3 +-
include/net/net_namespace.h | 3 +-
include/uapi/linux/magic.h | 1 +
init/main.c | 2 +
init/version.c | 5 +-
ipc/msgutil.c | 5 +-
ipc/namespace.c | 32 ++++----
kernel/nsproxy.c | 10 +--
kernel/pid.c | 5 +-
kernel/pid_namespace.c | 29 ++++----
kernel/user.c | 5 +-
kernel/user_namespace.c | 29 ++++----
kernel/utsname.c | 31 ++++----
net/Makefile | 2 -
net/core/net_namespace.c | 39 +++++-----
net/nonet.c | 26 -------
net/socket.c | 19 -----
36 files changed, 416 insertions(+), 430 deletions(-)
create mode 100644 fs/nsfs.c
create mode 100644 include/linux/ns_common.h
delete mode 100644 net/nonet.c
^ permalink raw reply [flat|nested] 12+ messages in thread
* [git pull] vfs.git, pile 2
@ 2017-03-02 12:38 Al Viro
0 siblings, 0 replies; 12+ messages in thread
From: Al Viro @ 2017-03-02 12:38 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel
orangefs fix + series of fs/namei.c cleanups from
me + VFS stuff coming from overlayfs tree.
The following changes since commit f1ef09fde17f9b77ca1435a5b53a28b203afb81c:
Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace (2017-02-23 20:33:51 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus
for you to fetch changes up to 653a7746fa2f5369985f5368ffc162b6510db6c8:
Merge remote-tracking branch 'ovl/for-viro' into for-linus (2017-03-02 06:41:22 -0500)
----------------------------------------------------------------
Al Viro (6):
namei: fold unlazy_link() into its sole caller
lookup_fast(): clean up the logics around the fallback to non-rcu mode
namei.c: fold the check for DCACHE_OP_REVALIDATE into d_revalidate()
namei.c: split unlazy_walk()
Merge branch 'work.namei' into for-linus
Merge remote-tracking branch 'ovl/for-viro' into for-linus
Amir Goldstein (4):
vfs: create vfs helper vfs_tmpfile()
vfs: deny fallocate() on directory
vfs: deny copy_file_range() for non regular files
vfs: wrap write f_ops with file_{start,end}_write()
Miklos Szeredi (5):
vfs: extract common parts of {compat_,}do_readv_writev()
vfs: pass type instead of fn to do_{loop,iter}_readv_writev()
vfs: use helpers for calling f_op->{read,write}_iter()
mm: use helper for calling f_op->mmap()
vfs: use helper for calling f_op->fsync()
Peter Zijlstra (1):
orangefs: Use RCU for destroy_inode
drivers/block/loop.c | 4 +-
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 2 +-
drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
fs/aio.c | 4 +-
fs/coda/file.c | 2 +-
fs/namei.c | 251 ++++++++++++++++++---------------
fs/open.c | 14 +-
fs/orangefs/super.c | 9 +-
fs/read_write.c | 130 ++++++++---------
fs/splice.c | 2 +-
fs/sync.c | 2 +-
include/linux/fs.h | 52 +++++--
ipc/shm.c | 4 +-
mm/mmap.c | 2 +-
mm/nommu.c | 4 +-
15 files changed, 261 insertions(+), 223 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread* [git pull] vfs.git pile 2
@ 2016-12-16 22:12 Al Viro
2016-12-18 2:49 ` Linus Torvalds
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2016-12-16 22:12 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel
In this pile:
* autofs-namespace series
* dedupe stuff
* more struct path constification
One conflict (xfs_reflink.c; resolved by taking this variant).
The following changes since commit 3e5de27e940d00d8d504dfb96625fb654f641509:
Linux 4.9-rc8 (2016-12-04 12:50:51 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus
for you to fetch changes up to 9763f7a4a5f7b1a7c480fa06d01b2bad25163c0a:
Merge branch 'work.autofs' into for-linus (2016-12-16 16:34:52 -0500)
----------------------------------------------------------------
Al Viro (21):
fsnotify: constify 'data' passed to ->handle_event()
fsnotify(): constify 'data'
constify fsnotify_parent()
fsnotify: constify the places working with ->f_path
audit_log_{name,link_denied}: constify struct path
constify get_dcookie() and friends
autofs: constify find_autofs_mount() callback
constify btrfs_mksubvol()
constify alloc_file()
quota: constify struct path in quota_on
namespace.c: constify struct path passed to a bunch of primitives
vfs: misc struct path constification
nfs_write_end(): fix handling of short copies
fix ceph_write_end()
fix gfs2_stuffed_write_end() on short copies
9p: saner ->write_end() on failing copy into non-uptodate page
exofs: don't mess with simple_write_{begin,end}
simple_write_end(): don't zero in short copy into uptodate
Merge branch 'work.write_end' into for-linus
Merge remote-tracking branch 'djwong/ocfs2-vfs-reflink-6' into for-linus
Merge branch 'work.autofs' into for-linus
Christoph Hellwig (1):
fs: try to clone files first in vfs_copy_file_range
Darrick J. Wong (10):
vfs: refactor clone/dedupe_file_range common functions
ocfs2: convert inode refcount test to a helper
ocfs2: add newlines to some error messages
ocfs2: prohibit refcounted swapfiles
ocfs2: budget for extent tree splits when adding refcount flag
ocfs2: don't eat io errors during _dio_end_io_write
ocfs2: always unlock when completing dio writes
ocfs2: fix bad pointer cast
ocfs2: charge quota for reflinked blocks
ocfs2: implement the VFS clone_range, copy_range, and dedupe_range features
Ian Kent (10):
vfs: change d_manage() to take a struct path
vfs: add path_is_mountpoint() helper
vfs: add path_has_submounts()
autofs: change autofs4_expire_wait()/do_expire_wait() to take struct path
autofs: change autofs4_wait() to take struct path
autofs: use path_is_mountpoint() to fix unreliable d_mountpoint() checks
autofs: use path_has_submounts() to fix unreliable have_submount() checks
vfs: remove unused have_submounts() function
autofs - constify misc struct path instances
autofs - dont hold spin lock over direct mount expire
Mickaël Salaün (1):
fs: Constify path_is_under()'s arguments
Documentation/filesystems/Locking | 2 +-
Documentation/filesystems/vfs.txt | 2 +-
arch/powerpc/oprofile/cell/spu_task_sync.c | 2 +-
drivers/oprofile/buffer_sync.c | 2 +-
fs/9p/vfs_addr.c | 15 +-
fs/autofs4/autofs_i.h | 5 +-
fs/autofs4/dev-ioctl.c | 10 +-
fs/autofs4/expire.c | 25 +-
fs/autofs4/root.c | 61 ++--
fs/autofs4/waitq.c | 13 +-
fs/btrfs/ctree.h | 3 -
fs/btrfs/file.c | 1 -
fs/btrfs/ioctl.c | 14 +-
fs/ceph/addr.c | 14 +-
fs/dcache.c | 40 +--
fs/dcookies.c | 4 +-
fs/exofs/inode.c | 68 ++---
fs/ext4/super.c | 4 +-
fs/file_table.c | 2 +-
fs/gfs2/aops.c | 4 +-
fs/internal.h | 2 +-
fs/libfs.c | 14 +-
fs/mount.h | 6 +
fs/namei.c | 17 +-
fs/namespace.c | 39 ++-
fs/nfs/file.c | 2 +-
fs/notify/dnotify/dnotify.c | 2 +-
fs/notify/fanotify/fanotify.c | 8 +-
fs/notify/fanotify/fanotify.h | 2 +-
fs/notify/fsnotify.c | 8 +-
fs/notify/inotify/inotify.h | 2 +-
fs/notify/inotify/inotify_fsnotify.c | 4 +-
fs/ocfs2/alloc.c | 7 +-
fs/ocfs2/aops.c | 31 +-
fs/ocfs2/file.c | 42 ++-
fs/ocfs2/file.h | 3 +
fs/ocfs2/inode.h | 6 +
fs/ocfs2/move_extents.c | 10 +-
fs/ocfs2/refcounttree.c | 464 ++++++++++++++++++++++++++++-
fs/ocfs2/refcounttree.h | 7 +
fs/ocfs2/xattr.c | 4 +-
fs/quota/dquot.c | 2 +-
fs/quota/quota.c | 4 +-
fs/read_write.c | 231 +++++++++++++-
fs/reiserfs/super.c | 4 +-
fs/statfs.c | 2 +-
fs/utimes.c | 2 +-
fs/xfs/xfs_file.c | 19 --
fs/xfs/xfs_reflink.c | 213 +------------
include/linux/audit.h | 2 +-
include/linux/dcache.h | 4 +-
include/linux/dcookies.h | 4 +-
include/linux/file.h | 2 +-
include/linux/fs.h | 12 +-
include/linux/fsnotify.h | 12 +-
include/linux/fsnotify_backend.h | 10 +-
include/linux/mount.h | 6 +-
include/linux/quota.h | 2 +-
include/linux/quotaops.h | 2 +-
kernel/audit.c | 4 +-
kernel/audit.h | 2 +-
kernel/audit_fsnotify.c | 10 +-
kernel/audit_tree.c | 2 +-
kernel/audit_watch.c | 8 +-
64 files changed, 1027 insertions(+), 503 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [git pull] vfs.git pile 2
2016-12-16 22:12 [git pull] vfs.git " Al Viro
@ 2016-12-18 2:49 ` Linus Torvalds
2016-12-18 3:26 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2016-12-18 2:49 UTC (permalink / raw)
To: Al Viro; +Cc: Linux Kernel Mailing List, linux-fsdevel
On Fri, Dec 16, 2016 at 2:12 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> In this pile:
> * autofs-namespace series
> * dedupe stuff
> * more struct path constification
When looking at the conflict, I looked at that part of
vfs_clone_file_prep_inodes(), and reacted to the insanity.
WTF?
/* Zero length dedupe exits immediately; reflink goes to EOF. */
if (*len == 0) {
if (is_dedupe) {
*len = 0;
return 0;
}
*len = isize - pos_in;
}
I'll just leave you to read through that part a bit more. Because
there are two completely insane things going on in that code sequence.
Linus
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [git pull] vfs.git pile 2
2016-12-18 2:49 ` Linus Torvalds
@ 2016-12-18 3:26 ` Al Viro
2016-12-18 3:34 ` Linus Torvalds
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2016-12-18 3:26 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux Kernel Mailing List, linux-fsdevel, Darrick J. Wong
[Darrick Cc'd]
On Sat, Dec 17, 2016 at 06:49:55PM -0800, Linus Torvalds wrote:
> On Fri, Dec 16, 2016 at 2:12 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > In this pile:
> > * autofs-namespace series
> > * dedupe stuff
> > * more struct path constification
>
> When looking at the conflict, I looked at that part of
> vfs_clone_file_prep_inodes(), and reacted to the insanity.
>
> WTF?
>
> /* Zero length dedupe exits immediately; reflink goes to EOF. */
> if (*len == 0) {
> if (is_dedupe) {
> *len = 0;
> return 0;
> }
> *len = isize - pos_in;
> }
>
> I'll just leave you to read through that part a bit more. Because
> there are two completely insane things going on in that code sequence.
One, AFAICS - pointless *len = 0 in case of is_dedupe. That's a counterpart of
/* Zero length dedupe exits immediately; reflink goes to EOF. */
if (len == 0) {
if (is_dedupe) {
ret = 0;
goto out_unlock;
}
len = isize - pos_in;
}
in mainline xfs_reflink_remap_range(). What else am I missing there?
I'm not thrilled with the calling conventions they'd used, and that
*len = 0; shouldn't have been slapped there (at a guess, by inertia from
the conversion of the chunk right before that one -
/* Are we going all the way to the end? */
isize = i_size_read(inode_in);
if (isize == 0) {
ret = 0;
goto out_unlock;
}
in mainline, needing *len = 0; after conversion), but I don't see what else
are you refering to in that snippet...
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [git pull] vfs.git pile 2
2016-12-18 3:26 ` Al Viro
@ 2016-12-18 3:34 ` Linus Torvalds
2016-12-18 4:06 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2016-12-18 3:34 UTC (permalink / raw)
To: Al Viro; +Cc: Linux Kernel Mailing List, linux-fsdevel, Darrick J. Wong
On Sat, Dec 17, 2016 at 7:26 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> One, AFAICS - pointless *len = 0 in case of is_dedupe.
Yup.
That part made sense in the original code where len and ret were
different. Now it just looks like a five-year old has been eating
crayons and paste.
> What else am I missing there?
I absolutely *abhor* this part:
*len = isize - pos_in;
because the whole code then depends on the overflow checking a few
lines down, and it's not at all obvious. We have not tested that
"pos_in" is smaller than "isize", even though the comment above the
"isize == 0" test inplies we did some kind of "past the end check" (we
did not).
The whole "depend on overflow checking" being nasty is particularly
true when that checking itself is damn subtle, and depends deeply on
the type of "*len" being unsigned and larger than "loff_t". Which in
turn is true, but it's all really nasty, and it's subtle. "loff_t" is
"long long", while "*len" is u64, and it's almost just luck that the
comparison does in fact end up unsigned.
So I think that code really needs a fair amount of loving.
Linus
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [git pull] vfs.git pile 2
2016-12-18 3:34 ` Linus Torvalds
@ 2016-12-18 4:06 ` Al Viro
2016-12-18 23:40 ` Darrick J. Wong
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2016-12-18 4:06 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux Kernel Mailing List, linux-fsdevel, Darrick J. Wong
On Sat, Dec 17, 2016 at 07:34:45PM -0800, Linus Torvalds wrote:
> > What else am I missing there?
>
> I absolutely *abhor* this part:
>
> *len = isize - pos_in;
>
> because the whole code then depends on the overflow checking a few
> lines down, and it's not at all obvious. We have not tested that
> "pos_in" is smaller than "isize", even though the comment above the
> "isize == 0" test inplies we did some kind of "past the end check" (we
> did not).
>
> The whole "depend on overflow checking" being nasty is particularly
> true when that checking itself is damn subtle, and depends deeply on
> the type of "*len" being unsigned and larger than "loff_t". Which in
> turn is true, but it's all really nasty, and it's subtle. "loff_t" is
> "long long", while "*len" is u64, and it's almost just luck that the
> comparison does in fact end up unsigned.
I agree, but that one is a straight move - exact same thing is there in
xfs_reflink.c counterpart in the current mainline.
> So I think that code really needs a fair amount of loving.
Indeed. Darrick, would you add a followup cleaning that up? It can be
done after the move to fs/read_write.c - no need to reorder/rebase that
thing. While we are at it, it might be better to turn the return value
into -E.../0/1, 0 being "no error, but nothing to do" and 1 - the normal
success case. That would get rid of using *len = 0 as signalling mechanism -
the caller would simply do
ret = vfs_..._inodes(.....);
if (ret <= 0)
goto out_unlock;
/* returned positive, we have work to do */
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [git pull] vfs.git pile 2
2016-12-18 4:06 ` Al Viro
@ 2016-12-18 23:40 ` Darrick J. Wong
0 siblings, 0 replies; 12+ messages in thread
From: Darrick J. Wong @ 2016-12-18 23:40 UTC (permalink / raw)
To: Al Viro; +Cc: Linus Torvalds, Linux Kernel Mailing List, linux-fsdevel
On Sun, Dec 18, 2016 at 04:06:06AM +0000, Al Viro wrote:
> On Sat, Dec 17, 2016 at 07:34:45PM -0800, Linus Torvalds wrote:
> > > What else am I missing there?
> >
> > I absolutely *abhor* this part:
> >
> > *len = isize - pos_in;
> >
> > because the whole code then depends on the overflow checking a few
> > lines down, and it's not at all obvious. We have not tested that
> > "pos_in" is smaller than "isize", even though the comment above the
> > "isize == 0" test inplies we did some kind of "past the end check" (we
> > did not).
> >
> > The whole "depend on overflow checking" being nasty is particularly
> > true when that checking itself is damn subtle, and depends deeply on
> > the type of "*len" being unsigned and larger than "loff_t". Which in
> > turn is true, but it's all really nasty, and it's subtle. "loff_t" is
> > "long long", while "*len" is u64, and it's almost just luck that the
> > comparison does in fact end up unsigned.
>
> I agree, but that one is a straight move - exact same thing is there in
> xfs_reflink.c counterpart in the current mainline.
Ok, fair enough. I thought it was ok but then I've spent so much time
staring at the reflink code it's good to have a fresh set of eyes. :)
I'll add a if (pos_in > isize) return -EINVAL there to make it more
explicit.
> > So I think that code really needs a fair amount of loving.
>
> Indeed. Darrick, would you add a followup cleaning that up? It can be
> done after the move to fs/read_write.c - no need to reorder/rebase that
> thing. While we are at it, it might be better to turn the return value
> into -E.../0/1, 0 being "no error, but nothing to do" and 1 - the normal
> success case. That would get rid of using *len = 0 as signalling mechanism -
> the caller would simply do
> ret = vfs_..._inodes(.....);
> if (ret <= 0)
> goto out_unlock;
> /* returned positive, we have work to do */
Sounds good. I'll post a cleanup patch once it goes through the
xfstests wringer.
--D
^ permalink raw reply [flat|nested] 12+ messages in thread
* [git pull] vfs.git pile 2
@ 2013-05-04 20:23 Al Viro
0 siblings, 0 replies; 12+ messages in thread
From: Al Viro @ 2013-05-04 20:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel
Assorted fixes, etc. Please, pull from
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus
Shortlog:
Al Viro (6):
do_mount(): fix a leak introduced in 3.9 ("mount: consolidate permission checks")
do_coredump(): don't wait for thaw if coredump has already been interrupted
more mode_t whack-a-mole...
kill fs/read_write.h
create_mnt_ns: unidiomatic use of list_add()
xtensa simdisk: fix braino in "xtensa simdisk: switch to proc_create_data()"
Geert Uytterhoeven (2):
nubus: Kill nubus_proc_detach_device()
sun3_scsi: add ->show_info()
Han Shen (1):
Removed unused typedef to avoid "unused local typedef" warnings.
James Hogan (3):
hostfs: remove "will unlock" comment
hostfs: move HOSTFS_SUPER_MAGIC to <linux/magic.h>
hostfs: use kmalloc instead of kzalloc
Jan Kara (1):
fs: Fix hang with BSD accounting on frozen filesystem
Syam Sidhardhan (1):
proc_devtree: Replace include linux/module.h with linux/export.h
Wei Yongjun (1):
vfs: use list_move instead of list_del/list_add
Yan, Zheng (1):
fs: remove dentry_lru_prune()
Diffstat:
arch/xtensa/platforms/iss/simdisk.c | 2 +-
drivers/hwmon/abx500.c | 2 +-
drivers/nubus/proc.c | 15 ---
drivers/scsi/sun3_NCR5380.c | 185 +++++++++++++++--------------------
drivers/scsi/sun3_scsi.c | 1 +
drivers/scsi/sun3_scsi.h | 2 +-
fs/compat.c | 2 -
fs/compat_ioctl.c | 1 -
fs/coredump.c | 9 +-
fs/dcache.c | 34 ++-----
fs/hostfs/hostfs_kern.c | 8 +-
fs/namespace.c | 7 +-
fs/proc/proc_devtree.c | 2 +-
fs/read_write.c | 5 +-
fs/read_write.h | 9 --
include/linux/fs.h | 7 ++
include/linux/nubus.h | 1 -
include/uapi/linux/magic.h | 1 +
kernel/acct.c | 7 +-
19 files changed, 124 insertions(+), 176 deletions(-)
delete mode 100644 fs/read_write.h
^ permalink raw reply [flat|nested] 12+ messages in thread* [git pull] vfs.git, pile 2
@ 2012-10-12 1:19 Al Viro
0 siblings, 0 replies; 12+ messages in thread
From: Al Viro @ 2012-10-12 1:19 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel
Stuff in this one - assorted fixes, lglock tidy-up, death to
lock_super(). There'll be a VFS pile tomorrow (with patches from Jeff Layton,
sanitizing getname() and related parts of audit and preparing for ESTALE
fixes), but I'd rather push the stuff in this one ASAP - some of the bugs
closed here are quite unpleasant. Please, pull from the usual place -
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git for-linus
Shortlog:
Al Viro (2):
MAX_LFS_FILESIZE definition for 64bit needs LL...
consitify do_mount() arguments
Arnd Bergmann (1):
vfs: bogus warnings in fs/namei.c
Hugh Dickins (1):
tmpfs,ceph,gfs2,isofs,reiserfs,xfs: fix fh_len checking
Lai Jiangshan (3):
lglock: remove unused DEFINE_LGLOCK_LOCKDEP()
lglock: make the per_cpu locks static
lglock: add DEFINE_STATIC_LGLOCK()
Marco Stornelli (7):
exofs: drop lock/unlock super
ext3: drop lock/unlock super
fat: drop lock/unlock super
hpfs: drop lock/unlock super
sysv: drop lock/unlock super
ufs: drop lock/unlock super
vfs: drop lock/unlock super
Richard W.M. Jones (1):
dup3: Return an error when oldfd == newfd.
Sasha Levin (2):
fs: prevent use after free in auditing when symlink following was denied
fs: handle failed audit_log_start properly
Diffstat:
fs/ceph/export.c | 18 ++++++++++++++----
fs/exofs/super.c | 4 ----
fs/ext3/super.c | 6 ------
fs/fat/dir.c | 4 ++--
fs/fat/fat.h | 5 +++--
fs/fat/inode.c | 5 +++--
fs/fat/namei_msdos.c | 26 +++++++++++++-------------
fs/fat/namei_vfat.c | 30 +++++++++++++++---------------
fs/file.c | 3 +++
fs/file_table.c | 2 +-
fs/gfs2/export.c | 4 ++++
fs/hpfs/super.c | 3 ---
fs/isofs/export.c | 2 +-
fs/namei.c | 3 ++-
fs/namespace.c | 12 ++++++------
fs/reiserfs/inode.c | 6 +++++-
fs/super.c | 23 -----------------------
fs/sysv/balloc.c | 18 +++++++++---------
fs/sysv/ialloc.c | 14 +++++++-------
fs/sysv/inode.c | 4 ++--
fs/sysv/super.c | 1 +
fs/sysv/sysv.h | 1 +
fs/ufs/balloc.c | 30 +++++++++++++++---------------
fs/ufs/ialloc.c | 16 ++++++++--------
fs/ufs/super.c | 21 +++++++++++----------
fs/ufs/ufs.h | 1 +
fs/xfs/xfs_export.c | 3 +++
include/linux/fs.h | 5 ++---
include/linux/lglock.h | 19 ++++++++-----------
include/linux/security.h | 12 ++++++------
kernel/audit.c | 2 ++
mm/shmem.c | 6 ++++--
security/capability.c | 4 ++--
security/security.c | 4 ++--
security/selinux/hooks.c | 4 ++--
security/smack/smack_lsm.c | 4 ++--
security/tomoyo/common.h | 2 +-
security/tomoyo/mount.c | 5 +++--
security/tomoyo/tomoyo.c | 4 ++--
39 files changed, 166 insertions(+), 170 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-02 12:40 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-15 23:51 [git pull] vfs.git pile 2 Al Viro
2014-12-16 13:33 ` Al Viro
2014-12-16 18:06 ` Al Viro
-- strict thread matches above, loose matches on Subject: below --
2017-03-02 12:38 [git pull] vfs.git, " Al Viro
2016-12-16 22:12 [git pull] vfs.git " Al Viro
2016-12-18 2:49 ` Linus Torvalds
2016-12-18 3:26 ` Al Viro
2016-12-18 3:34 ` Linus Torvalds
2016-12-18 4:06 ` Al Viro
2016-12-18 23:40 ` Darrick J. Wong
2013-05-04 20:23 Al Viro
2012-10-12 1:19 [git pull] vfs.git, " Al Viro
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).