Message-ID: <344eb09a0706200209j30ccad0k17bc994b6e637988@mail.gmail.com>
Date:	Wed, 20 Jun 2007 14:39:34 +0530
From:	"Bharata B Rao" <bharata.rao@...il.com>
To:	"Jan Blunck" <jblunck@...e.de>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	bharata@...ux.vnet.ibm.com
Subject: Re: [RFC PATCH 1/4] Union mount documentation.

On 6/20/07, Jan Blunck <jblunck@...e.de> wrote:
> On Wed, 20 Jun 2007 11:21:57 +0530, Bharata B Rao wrote:
> <snip>
> Well done. I like your approach much more than the simple chaining of
> dentries. When I told you about the idea of maintaining a list of
> <dentry,vfsmount> objects I always though about one big structure for all
> the layers of an union. Smaller objects that only point to the next layer
> seem to be better but make the search for the topmost layer impossible.
> You should maintain a reference to the topmost struct union_mount though.

Even in our last version I didn't understand clearly why you had
pointers from the bottom layers to the topmost layer. Could you please
explain under what circumstances there needs to be a bottom to top
traversal ?

>
> > +5. Union stack: destroying
> > +--------------------------
> > +In addition to storing the union_mounts in a hash table for quick
> > lookups, +they are also stored as a list, headed at vsmount->mnt_union.
> > So, all +union_mounts that occur under a vfsmount (starting from the
> > mountpoint +followed by the subdir unions) are stored within the
> > vfsmount. During +umount (specifically, during the last mntput()), this
> > list is traversed +to destroy all union stacks under this vfsmount. +
> > +Hence, all union stacks under a vfsmount continue to exist until the
> > +vfsmount is unmounted. It may be noted that the union_mount structure
> > +holds a reference to the current dentry also. Becasue of this, for
> > +subdir unions, both the top and bottom level dentries become pinned
> > +till the upper layer filesystem is unmounted. Is this behaviour
> > +acceptable ? Would this lead to a lot of pinned dentries over a period
> > +of time ? (CHECK) If we don't do this, the top layer dentry might go
> > +out of cache, during which time we have no means to release the
> > +corresponding union_mount and the union_mount becomes stale. Would it
> > +be necessary and worthwhile to add intelligence to prune_dcache() to
> > +prune unused union_mounts thereby releasing the dentries ? +
> > +As noted above, we hold the refernce to current dentry from union_mount
> > +but don't get a reference to the corresponding vfsmount. We depend on
> > +the user of the union stack to hold the reference to the topmost
> > vfsmount +until he is done with the stack traversal. Not holding a
> > reference to the +top vfsmount from within union_mount allows us to free
> > all the union_mounts +from last mntput of the top vfsmount. Is this
> > approach acceptable ? +
> > +NOTE: union_mount structures are part of two lists: the hash list for
> > +quick lookups and a linked list to aid the freeing of these structures
> > +during unmount.
>
> This must changed. This is the only reason why the dentry chaining
> approach was so complex. You need a way to get rid of all unused dentries
> in a union.

The second list headed at mnt->mnt_union was added precisely  to get
rid of all the union_mounts under a vfsmount at umount time. So umount
is the time to destroy the union stack.
>
> Besides that, I wonder why you left out the rest of my code? The readdir,
> whiteout and copyup parts are orthogonal to the code for maintaining the
> union structure itself. I just rewrote most of it myself to use functions
> like follow_union_down() etc to get rid of the dentry chaining in the long
> run.

The idea was to start simple, get some feedback and concensus and add
features after that. Some of the feedback I got from our last two
posts was that the code was too complex and big to review and we had
so many patches. So this time I have started with the bare minimum so
that it becomes easier for the reviewers. I plan to add copyup and
whiteout only when there is an agreement that this approach of
unioning is acceptable.

And about readdir, I digressed from your approach a bit and made
readdir cache persistant across readdir()/getdents() calls. Also, made
readdir on union mounted directories filesystem independent unlike our
earlier approach. But again this breaks lseek as I have noted, which
needs to be fixed.

Regards,
Bharata.
-- 
"Men come and go but mountains remain" -- Ruskin Bond.
-
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/

Powered by blists - more mailing lists