[RFC][patch series] vfsmount gutting

[RFC][patch series] vfsmount gutting

[Al Viro]

	OK, that's something I wanted to do for a long time - struct vfsmount
contains too much stuff that is strictily VFS-internal and not needed by
anything outside of a very small subset of VFS, at that.  As the matter of
fact, only 3 fields, 1 of them redundant, are used by code outside of that
area - it's mnt_flags ("how it's mounted"), mnt_root ("what dentry tree
is mounted here", assign-once thing) and mnt_sb (always equal to
mnt_root->d_sb, also assign-once, might or might not be not needed).

	What I've done in vfs.git#vfsmount-guts is massive trimming of
struct vfsmount.  It's a long massage session for fs/namespace.c and
friends.  Here's what happens there:

	* struct vfsmount is embedded into a VFS-private structure.  The
best name I'd been able to come up with was struct mount and yes, it would
be more natural the other way round.  Since all instances of struct vfsmount
come from alloc_vfsmnt(), it's safe - they can't be static, they can't be
on stack, they can't be embedded into other structures or arrays.
	* said structure is defined in fs/mounts.h.  All fields of struct
vfsmount except for these 3 had been moved to struct mount.  Very few files
include that one.
	* fs/namespace.c and fs/pnode.c internals are dealing with struct
mount * instead of struct vfsmount *; the public API is unchanged and
still gets struct vfsmount *, of course.  We use container_of() to get to
the corresponding struct mount * (see real_mount() in fs/mounts.h).
	* ->mnt_parent and ->mnt_master point to struct mount *; it's
more convenient that way.
	* struct mnt_namespace became opaque - its definition is in the
same fs/mounts.h.  ->root points to struct mount * now (that, BTW, is
what had lead to grep over the tree that found breakage in apparmor).
	* proc/*/mounts-related code got moved to separate file -
fs/proc_namespace.c; essentially it's a part of procfs, but its ties to
fs/namespace.c are much stronger than those to the rest of fs/proc/*.
Interface boundary shifted a bit, with IMO cleaner separation as the
result.

	Filesystem code is unchanged; it still gets pointers to struct
vfsmount *, it's just that this structure is less cluttered now and
IMO it makes a lot more sense that way.

	What is missing:
1) Review.  As far as I can see on local testing, the thing works, but extra
eyes would obviously be very welcome.
2) Documentation of the vfsmount ex-guts.  This stuff is dealt with by
very compact area now, so it really can be sanely documented.  In progress...
3) layout optimizations wrt holes, cacheline boundaries, etc.  It's not
too far from what we used to have before, but it'll need to be dealt with.
4) *possibly* a removal of ->mnt_sb.  I'm not sure about that one; it
is redundant (for all vfsmounts ->mnt_sb == ->mnt_root->d_sb and they are
assigned at the same point, before anything can see that vfsmount) and
it's not as if it had been used on a lot of hot paths, but...  Note, BTW,
that in a lot of cases we have path->dentry->d_sb spelled as path->mnt->mnt_sb;
these don't really need ->mnt_sb, of course.  Another class of users is
something along the lines of configfs_mount->mnt_sb->s_root.  What it wants
to be is configfs_mount->mnt_root - these internal vfsmounts have roots
set to that fs in question, so that's a straightforward optimization.  Hell
knows...  It's a separate series in any case.

	I'm going to put that series into -next; if anybody sees problems
with it, please yell.

	I'm *NOT* mailbombing fsdevel with that - it's a long series and
if somebody wants a private mailbomb, that can always be arranged.  Or you
could grab git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git
#vfsmount-guts and use git format-patch locally...

Al, off to review Miklos' series now...

Re: [RFC][patch series] vfsmount gutting

[Linus Torvalds]

On Wed, Dec 7, 2011 at 5:02 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>        I'm *NOT* mailbombing fsdevel with that - it's a long series and
> if somebody wants a private mailbomb, that can always be arranged.  Or you
> could grab git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git
> #vfsmount-guts and use git format-patch locally...

gitweb is also not horrible for things like this. You can point your browser at

  http://git.kernel.org/?p=linux/kernel/git/viro/vfs.git;a=shortlog;h=refs/heads/vfsmount-guts

and click on the top-most commitdiff. Then you can see follow the rest
by just clicking on "parent", and you get the nicely colorized diff
with changelog entries etc. Or you can pick-and-choose based on
shortlog name.

So linear series like these can be looked at even without direct git
access. Sure, you'll get that "generating.." thing if the cache is
cold, and it all depends on how fast the net is, but it's not
necessarily any worse than being mail-bombed.

                           Linus

Re: [RFC][patch series] vfsmount gutting

[Matthew Wilcox]

On Thu, Dec 08, 2011 at 01:02:52AM +0000, Al Viro wrote:
> 	OK, that's something I wanted to do for a long time - struct vfsmount
> contains too much stuff that is strictily VFS-internal and not needed by
> anything outside of a very small subset of VFS, at that.  As the matter of
> fact, only 3 fields, 1 of them redundant, are used by code outside of that
> area - it's mnt_flags ("how it's mounted"), mnt_root ("what dentry tree
> is mounted here", assign-once thing) and mnt_sb (always equal to
> mnt_root->d_sb, also assign-once, might or might not be not needed).

If only these 3 fields are used, why not make the definition of struct
vfsmount entirely hidden, and add accessor functions for those three?

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [RFC][patch series] vfsmount gutting

[Al Viro]

On Thu, Dec 08, 2011 at 05:19:57AM -0700, Matthew Wilcox wrote:
> On Thu, Dec 08, 2011 at 01:02:52AM +0000, Al Viro wrote:
> > 	OK, that's something I wanted to do for a long time - struct vfsmount
> > contains too much stuff that is strictily VFS-internal and not needed by
> > anything outside of a very small subset of VFS, at that.  As the matter of
> > fact, only 3 fields, 1 of them redundant, are used by code outside of that
> > area - it's mnt_flags ("how it's mounted"), mnt_root ("what dentry tree
> > is mounted here", assign-once thing) and mnt_sb (always equal to
> > mnt_root->d_sb, also assign-once, might or might not be not needed).
> 
> If only these 3 fields are used, why not make the definition of struct
> vfsmount entirely hidden, and add accessor functions for those three?

Code churn from hell, for starters.  And cost of non-inlined calls on some
fairly sensitive paths, but that's secondary.

It might turn out to be feasible, but the price is likely to include several
method prototype changes affecting filesystems all over the place.  E.g.
I really suspect that ->show_options() taking vfsmount had been a mistake.
Mine, at that.  It ought to take root dentry of (sub)tree instead.  No,
passing struct super_block * will _not_ work; the things that share superblock
between several mounts really want to know e.g. which of the snapshots it
is (nilfs2), etc.  The same probably goes for the rest of ->show...() methods.

Hell knows; if we end up in a place where what you said would be doable -
great, but I'd prefer to pull what can be pulled without messing with
fs code first.  Besides, this separation really makes sense - it's
"what part of fs this vfsmount refers to" vs. "how vfsmounts are connected
into mount trees".

Re: [RFC][patch series] vfsmount gutting

[James Morris]

>       What is missing:
> 1) Review.  As far as I can see on local testing, the thing works, but extra
> eyes would obviously be very welcome.

I've been through the code and not seen any issues, and I'm also running 
it on my dev box with SELinux on, with no apparent problems.

If anyone has the SELinux testsuite set up, it would be good to run it 
with these patches applied.



- James
-- 
James Morris
<jmorris@namei.org>

Re: [RFC][patch series] vfsmount gutting

[Tetsuo Handa]

Tried vfs.git#vfsmount-guts as of f939b0ff "vfs: trim includes a bit".
Regarding TOMOYO part, I got one build error and got no runtime problems.

[PATCH] TOMOYO: Fix build error.

Include linux/magic.h for PROC_SUPER_MAGIC and SOCKFS_MAGIC.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

--- a/security/tomoyo/realpath.c
+++ b/security/tomoyo/realpath.c
@@ -5,6 +5,7 @@
  */
 
 #include "common.h"
+#include <linux/magic.h>
 
 /**
  * tomoyo_encode2 - Encode binary string to ascii string.