Re: [RFD 1/4] Pass no useless nameidata to the create, lookup, and permission IOPs

[Erez Zadok <ezk@cs.sunysb.edu>]

In message <20070626231541.697783295@suse.de>, jjohansen@suse.de writes:
> The create, lookup, and permission inode operations are all passed a
> full nameidata.  This is unfortunate because in nfsd and the mqueue
> filesystem, we must instantiate a struct nameidata but cannot provide
> all of the same information that a regular lookup would provide.  The
> unused fields take up space on the stack, but more importantly, it is
> not obvious which fields have meaningful values and which don't, and so
> things might easily break.
> 
> This patch introduces struct nameidata2 with only the fields that make
> sense independent of an actual lookup, and uses that struct in those
> places where a full nameidat is not needed.

I agree w/ Trond that a better name is needed other than 'nameidata2',
esp. for something that's a sub-structure (perhaps start it with a '__'?)

These changes would probably help stackable file systems (e.g., eCryptfs and
esp. Unionfs) a lot, b/c stackable f/s often call the lower f/s to lookup
files and such; and in most cases, we just need to pass the intent down, not
the full VFS-level state info.

> +/**
> + * Fields shared between nameidata and nameidata2 -- nameidata2 could
> + * simply be embedded in nameidata, but then the vfs code would become
> + * cluttered with dereferences.
> + */
> +#define __NAMEIDATA2				\
> +	struct dentry	*dentry;		\
> +	struct vfsmount *mnt;			\
> +	unsigned int	flags;			\
> +						\
> +	union {					\
> +		struct open_intent open;	\
> +	} intent;

Perhaps it is also time to put the dentry + mnt into a single struct path?
It's a small change, but it emphasizes that the two items here, dentry+mnt,
really define a single path to be passed around:

#define __NAMEIDATA			\
	struct path path;		\
	unsigned int flags;		\
	...

Of course, you'll have to change instances of nd->dentry to nd->path.dentry
and so on.

Erez.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/