[lkml]   [2013]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] mm: cache largest vma
Ugh. This patch makes me angry. It looks way too ad-hoc.

I can well imagine that our current one-entry cache is crap and could
be improved, but this looks too random. Different code for the
CONFIG_MMU case? Same name, but for non-MMU it's a single entry, for
MMU it's an array? And the whole "largest" just looks odd. Plus why do
you set LAST_USED if you also set LARGEST?

Did you try just a two- or four-entry pseudo-LRU instead, with a
per-thread index for "last hit"? Or even possibly a small fixed-size
hash table (say "idx = (add >> 10) & 3" or something)?

And what happens for threaded models? Maybe we'd be much better off
making the cache be per-thread, and the flushing of the cache would be
a sequence number that has to match (so "vma_clear_cache()" ends up
just incrementing a 64-bit sequence number in the mm)?

Basically, my complaints boil down to "too random" and "too
specialized", and I can see (and you already comment on) this patch
being grown with even *more* ad-hoc random new cases (LAST, LARGEST,
MOST_USED - what's next?). And while I don't know if we should worry
about the threaded case, I do get the feeling that this ad-hoc
approach is guaranteed to never work for that, which makes me feel
that it's not just ad-hoc, it's also fundamentally limited.

I can see us merging this patch, but I would really like to hear that
we do so because other cleaner approaches don't work well. In
particular, pseudo-LRU tends to be successful (and cheap) for caches.


 \ /
  Last update: 2013-11-03 20:01    [W:0.192 / U:5.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site