Re: [patch] fs: shrink struct dentry

[Nick Piggin <npiggin@suse.de>]

On Wed, Dec 10, 2008 at 06:53:21AM +0000, Al Viro wrote:
> On Wed, Dec 10, 2008 at 07:00:48AM +0100, Nick Piggin wrote:
> > OK, this is a new patch with updated changelog, and slightly tweaked
> > change to dcookie subsystem (now we'll *never* do any more hash lookups
> > on dcookie lookup, ignoring that "fast" dcookie lookups would effectively
> > mean my previous version wouldn't have done any more lookups anyway).
> 
> I agree with reordering part, and I don't have too serious objections
> against the rest, but...
> 
> Just what the bleeding fsck is that stuff doing to paths?

Honestly, I tried to look at as little dcookie / oprofile code as
possible... These are good questions for the oprofile people cced,
though.

 
> a) get two instances of fs mounted somewhere (or just have a binding, or
> have two namespaces) and you'll get very funny results from dcookie -
> it'll stick with vfsmount it had run into originally.  Nevermind that
> it might be not reachable for you.  Or reachable for anyone, for that
> matter, since the namespace it had been in might be long gone by now.

Good question. What's the high level requirement? To look up a path
name from a lighter-weight data item? Should it rather be a pcookie
subsystem? Anyway, whatever the case, it really must create and
manage its own data structures to perform the relevant mappings,
rather than add fields to existing structures, I think.

 
> b) when is it evicted, anyway?  We are stuck with a bunch of randomly
> selected pinned dentries and vfsmounts...

When oprofile daemon stops sampling, AFAIKT. Yes there will be
randomly pinned executable files until then.

 
> c) may I have whatever dcookie_user inventor had been smoking?
> struct dcookie_user {
>         struct list_head next;
> };
> with a cyclic list going through those and the only use of that list
> is "is it empty?" checks.  That's one hell of a way to implement a
> counter, of course, but...

Heh. Maybe they had some grand plans for it.

> Anyway, I'll put it into VFS queue.

Thanks.