Messages in this thread | | | Date | Wed, 15 Jan 2014 12:47:35 +0400 | From | Vladimir Davydov <> | Subject | Re: [PATCH 1/5] mm: vmscan: shrink all slab objects if tight on memory |
| |
On 01/15/2014 02:14 AM, Andrew Morton wrote: > On Tue, 14 Jan 2014 11:23:30 +0400 Vladimir Davydov <vdavydov@parallels.com> wrote: > >> On 01/14/2014 03:05 AM, Andrew Morton wrote: >>> That being said, I think I'll schedule this patch as-is for 3.14. Can >>> you please take a look at implementing the simpler approach, send me >>> something for 3.15-rc1? >> IMHO the simpler approach (Glauber's patch) is not suitable as is, >> because it, in fact, neglects the notion of batch_size when doing low >> prio scans, because it calls ->scan() for < batch_size objects even if >> the slab has >= batch_size objects while AFAIU it should accumulate a >> sufficient number of objects to scan in nr_deferred instead. > Well. If you mean that when nr-objects=large and batch_size=32 and > total_scan=33, the patched code will scan 32 objects and then 1 object > then yes, that should be fixed.
I mean if nr_objects=large and batch_size=32 and shrink_slab() is called 8 times with total_scan=4, we can either call ->scan() 8 times with nr_to_scan=4 (Glauber's patch) or call it only once with nr_to_scan=32 (that's how it works now). Frankly, after a bit of thinking I am starting to doubt that this can affect performance at all provided the shrinker is implemented in a sane way, because as you've mentioned shrink_slab() is already a slow path. It seems I misunderstood the purpose of batch_size initially: I though we need it to limit the number of calls to ->scan(), but now I guess the only purpose of it is limiting the number of objects scanned in one pass to avoid latency issues. But then another question arises - why do you think the behavior you described above (scanning 32 and then 1 object if total_scan=33, batch_size=32) is bad? In other words why can't we make the scan loop look like this:
while (total_scan > 0) { unsigned long ret; unsigned long nr_to_scan = min(total_scan, batch_size);
shrinkctl->nr_to_scan = nr_to_scan; ret = shrinker->scan_objects(shrinker, shrinkctl); if (ret == SHRINK_STOP) break; freed += ret;
count_vm_events(SLABS_SCANNED, nr_to_scan); total_scan -= nr_to_scan;
cond_resched(); }
?
Thanks.
| |