Re: [patch][rfc] fs: shrink struct dentry

[Nick Piggin <npiggin@suse.de>]

On Mon, Dec 01, 2008 at 07:38:18PM +0000, John Levon wrote:
> On Mon, Dec 01, 2008 at 07:04:55PM +0100, Nick Piggin wrote:
> 
> > On Mon, Dec 01, 2008 at 05:51:13PM +0000, John Levon wrote:
> > > On Mon, Dec 01, 2008 at 09:33:43AM +0100, Nick Piggin wrote:
> > > 
> > > > I then got rid of the d_cookie pointer. This shrinks it to 192 bytes. Rant:
> > > > why was this ever a good idea? The cookie system should increase its hash
> > > > size or use a tree or something if lookups are a problem.
> > > 
> > > Are you saying you've made this change without even testing its
> > > performance impact?
> > 
> > For oprofile case (maybe if you are profiling hundreds of vmas and
> > overflow the 4096 byte hash table), no. That case is uncommon and
> > must be fixed in the dcookie code (as I said, trivial with changing
> > data structure). I don't want this pointer in struct dentry
> > regardless of a possible tiny benefit for oprofile.
> 
> Don't you even have a differential profile showing the impact of
> removing d_cookie? This hash table lookup will now happen on *every*
> userspace sample that's processed. That's, uh, a lot.

I don't know what you mean by every sample that's processed, but
won't the hash lookup only happen for the *first* time that a given
name is asked for a dcookie (ie. fast_get_dcookie, which, as I said,
should actually be moved to fs/dcookies.c).

If get_dcookie is called "a lot" of times, then this profiling code
is broken anyway. There is a global mutex in that function. It's bad
enough that it takes mmap_sem and does find_vma...


> (By all means make your change, but I don't get how it's OK to regress
> other code, and provide no evidence at all as to its impact.)

Tradeoffs are made all the time. This is obviously a good one, and
I provided evidence of the impact of the improvement in the common
case. I also acknowledge it can slow down the uncommon case, but
showed ways that can easily be improved. Do you want me to just try
to make an artificial case where I mmap thousands of tiny shared
libraries and try to overflow the hash and try to detect a difference?

Did you add d_cookie? If so, then surely at the time you must have
justified that with some numbers to show a significant improvement
to outweigh the clear downsides. Care to share? Then I might be able
to just reuse your test case.