Matthew Dobson wrote: > Andrew Morton wrote: > >>Matthew Dobson wrote: >> >> >>>Since shrink_slab() currently returns 0 no matter what happens, >>>I changed it to return the number of slab pages freed. >> >> >>A sane cleanup, but it conflicts with vmscan-notice-slab-shrinking.patch, >>which returns a different thing from shrink_slab() in order to account for >>slab reclaim only causing internal fragmentation and not actually freeing >>pages yet. >> >>vmscan-notice-slab-shrinking.patch isn't quite complete yet, but we do need >>to do something along these lines. I need to get back onto it. > > > I hadn't seen that patch... These can coexist fairly easily, though. I > can change my patch to zero and then read current->reclaimed_slab *outside* > the call to shrink_slab. It unfortunately shrinks less lines of code, and > leaves the hack that is current->reclaimed_slab exposed to more than one > function, but... > > How's this look? > > mcd@arrakis:~/linux/patches $ diffstat remove-reclaim_state.patch > include/linux/sched.h | 3 +-- > include/linux/swap.h | 8 -------- > mm/page_alloc.c | 6 ------ > mm/slab.c | 3 +-- > mm/vmscan.c | 21 ++++----------------- > 5 files changed, 6 insertions(+), 35 deletions(-) > > -Matt Ok. Now I'm getting REALLY crazy. I noticed that try_to_free_pages() & balance_pgdat() (the two callers of shrink_slab()) do something really dumb: they set sc.nr_scanned to 0, call shrink_slab() with sc.nr_scanned as a pass-by-value parameter, then do total_scanned += sc.nr_scanned, despite knowing damn well that sc.nr_scanned = 0, since _they just set it to that_. I doubt we really want to be constantly incrementing total_scanned by 0. We either don't want to update it, or we want to update it by the value scanned gets in shrink_slab(). So, you're probably asking yourself, what is the point of all this rambling? What if we change shrink_slab() to take a struct scan_control * instead of an unsigned long for it's first argument? Then we can _actually_ update nr_scanned AND nr_reclaimed directly inside of shrink_slab() instead of trying to pass all this data back to the two partially brain-dead callers. Updated to 2.6.12-rc3-mm2, which includes vmscan-notice-slab-shrinking.patch. Look at all those -'s!! :) mcd@arrakis:~/linux/patches $ diffstat remove-reclaim_state.patch include/linux/sched.h | 3 +-- include/linux/swap.h | 8 -------- mm/page_alloc.c | 6 ------ mm/slab.c | 4 ++-- mm/vmscan.c | 36 ++++++++++++------------------------ 5 files changed, 15 insertions(+), 42 deletions(-) Running it through a build & boot test now. I'll let you know if it barfs. -Matt