Messages in this thread | | | Date | Wed, 20 Jun 2007 14:39:34 +0530 | From | "Bharata B Rao" <> | Subject | Re: [RFC PATCH 1/4] Union mount documentation. |
| |
On 6/20/07, Jan Blunck <jblunck@suse.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |