Re: [RFC PATCH 1/4] Union mount documentation.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/20/07, Jan Blunck <[email protected]> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux