On Tue, 2009-03-31 at 18:38 -0400, Trond Myklebust wrote: > On Tue, 2009-03-31 at 14:40 -0700, Linus Torvalds wrote: > > > > On Wed, 11 Mar 2009, Trond Myklebust wrote: > > > > > > This patch therefore defines a VFS helper function that sets up a temporary > > > mount namespace to represent the server namespace, and has the current > > > task pivot into that prior to doing the path lookup. Upon completion, it > > > pivots back into the original namespace, and destroys the private one. > > > > This is disgusting. > > > > Why don't you just create the namespace once? > > > > Also, why do you need that disgusting pivot thing, when we could instead > > trivially just add a > > > > struct filesystem *fs; > > > > into the nameidata, and then we can initialize it to > > > > nd->fs = current->fs > > > > and make all the path walkers use that. > > > > Or we could even try to clean up that horrid AT_FDCWD mess, and drop the > > whole "dfd" passing to "do_path_lookup()", and instead do > > > > rwlock_t *lock; > > struct path *root, *pwd; > > > > and do > > > > nd->lock = ¤t->fs->lock; > > nd->root = ¤t->fs->root; > > nd->pwd = ¤t->fs->pwd; > > > > to initialize things. Then drop dfd as an argument entirely from > > path_lookup_open() and do_path_lookup(), and just have the caller > > initialize the nameidata (the only caller that doesn't use fs->pwd > > currently is do_filp_open(), which takes that 'dfd' and could just > > initialize nd->pwd to the right thing. > > I'd be fine with that. > > > I dunno. This is very Al Viroish country, but I really hate how your patch > > looks. 99% of it is just working around the fact that you want some very > > _slight_ differences to how that special '/' thing is handled. > > No. The main purpose is to able to look up and walk down an NFSv4 mount > path, which is a path on the _server_'s file/directory namespace. > In order to be able to follow any valid mount path that the user > supplies, we need the ability to follow symlinks, cross mount points and > even cross NFSv4 referrals (i.e. afs-like junctions that point to paths > on a different server). > > Under normal path walking we use the 'follow_link()' method of > autogenerating mountpoints (see > Documentation/filesystems/automount-support.txt) when we come across > mount points or referrals on the server. If you are walking a path that > is not supposed to be visible to user processes, you therefore need a > private namespace. > > > It's not worth doing these kinds of hacks for that. > > > > And I think it's positively _wrong_ to have a function that creates and > > destroys the whole "struct fs_struct" and a namespace for just one call. > > Even if you don't think it's at all performance-critical, the interface is > > too damn ugly. Have separate "create/destroy context" functions, so that > > you _can_ do it just once, and have multiple calls in between. > > That can probably be done, but the main reason for having the namespace > was to be able, once the sys_mount() is complete, to garbage collect and > get rid of those autogenerated mount points that are not user visible. I meant to say "...the main reason for destroying the namespace"... Cheers Trond -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
