linux-kernel - Re: [PATCH 19/38] union-mount: Introduce union

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100804194749.GA2134@shell>
Date:	Wed, 4 Aug 2010 15:47:50 -0400
From:	Valerie Aurora <vaurora@...hat.com>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	viro@...iv.linux.org.uk, jblunck@...e.de, hch@...radead.org,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 19/38] union-mount: Introduce union_dir structure and basic operations

On Wed, Aug 04, 2010 at 04:51:31PM +0200, Miklos Szeredi wrote:
> On Tue, 15 Jun 2010, Valerie Aurora wrote:
> > This patch adds the basic structures and operations of VFS-based union
> > mounts (but not the ability to mount or lookup unioned file systems).
> > Each directory in a unioned file system has an associated union stack
> > created when the directory is first looked up.  The union stack is a
> > union_dir structure kept in a hash table indexed by mount and dentry
> > of the directory; thus, specific paths are unioned, not dentries
> > alone.  The union_dir keeps a pointer to the upper path and the lower
> > path and can be looked up by either path.  Currently only two layers
> > are supported, but the union_dir struct is flexible enough to allow
> > more than two layers.
> > 
> > This particular version of union mounts is based on ideas by Jan
> > Blunck, Bharata Rao, and many others.
> > 
> > Signed-off-by: Valerie Aurora <vaurora@...hat.com>
> > ---
> 
> 
> > --- a/include/linux/dcache.h
> > +++ b/include/linux/dcache.h
> > @@ -100,7 +100,9 @@ struct dentry {
> >  	struct hlist_node d_hash;	/* lookup hash list */
> >  	struct dentry *d_parent;	/* parent directory */
> >  	struct qstr d_name;
> > -
> > +#ifdef CONFIG_UNION_MOUNT
> > +	struct union_dir *d_union_dir;	/* head of union stack */
> > +#endif
> 
> This botches the carefully tuned length of struct dentry.  At least a
> FIXME comment needs to be added that this is something to be
> addressed.

Okay, added.

> Why was the hash table concept dropped?  The header comment still
> talks about that?

Simply, Al Viro didn't like it.  But note that the current
implementation still uses part of the hash table solution.  You still
have union_dir structures external to dentries for the read-only
layers of the stack.  The change is from Al's observation that the
topmost dentry could only be part of one stack.  Why do a lookup on
the topmost dentry when you could keep an pointer to the stack in the
dentry itself and skip the lookup?  Once you have the head of the
stack, you don't need lookup for the rest of it.  This eliminates all
the lookup machinery and the union hash table lock, which seems like a
big win to me.

The biggest drawback of the hash table in my mind was that it
introduced a new global synchronization point in lookup.  Making it go
fast would be dcache lookup optimization all over again.

Thanks,

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