Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro <viro@ZenIV.linux.org.uk>]

On Wed, Apr 02, 2008 at 11:11:39PM +0200, Miklos Szeredi wrote:

> It is an interface with at least 3 callers at the moment: syscalls,
> nfsd and ecryptfs and unionfs in the future. It is an interface
> because all external accesses to the filesystem *are* done through the
> namespace and not directly on filesystem internals.
> 
> Such direct access would be conceivable, but I don't think we should
> base the design on theoretically possible uses.  If those uses appear,
> we can change the interface to fit that.
> 
> This "move everything to callers" thing is wrong because it just
> results in bloat and bugs, none of which is desirable in the kernel.

I disagree.  First of all, clear separation between operations on
_filesystem_, which should all be namespace-agnostic and things
that depend on vfsmount is a Good Thing(tm).  Think of that as
of separation between server (superblock and everything related
to it, starting with dentry tree) and clients; mixing those is a
bloody bad idea.

What's more, I'm not at all convinced that for nfsd it's the right
set of checks, to start with.  The same goes for future users.

As for ecryptfs, looking at their "lower_mnt" thing...  I'd say that
it's a nonsense.  For one thing, duplicating a reference into ever
dentry out there (and it's simply duplicated) makes no sense whatsoever.
For another...  I'm not at all sure that remount of the underlying
vfsmount r/o *should* take that sucker read-only.  And if it should,
it's clearly an action that should have a visible effect on superblock
flags of ecryptfs.

	Incidentally, looking at ecryptfs open(), WTF is protecting
crypt_stat->flags?  We take crypt_stat->cs_mutex, do nothing but
check-and-modify of ->flags under it, then drop and several lines
later do crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED); with no ->cs_mutex
held...