| Message ID | 20170815153010.e3cfc177af0b2c0dc421b84c@linux-foundation.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Andrew Morton (2017-08-15 23:30:10) > On Sat, 12 Aug 2017 12:34:37 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > Some shrinkers may only be able to free a bunch of objects at a time, and > > so free more than the requested nr_to_scan in one pass. Account for the > > extra freed objects against the total number of objects we intend to > > free, otherwise we may end up penalising the slab far more than intended. > > > > ... > > > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -398,6 +398,7 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl, > > break; > > freed += ret; > > > > + nr_to_scan = max(nr_to_scan, ret); > > count_vm_events(SLABS_SCANNED, nr_to_scan); > > total_scan -= nr_to_scan; > > scanned += nr_to_scan; > > Well... kinda. But what happens if the shrinker scanned more objects > than requested but failed to free many of them? Of if the shrinker > scanned less than requested? > > We really want to return nr_scanned from the shrinker invocation. > Could we add a field to shrink_control for this? Yes, that will work better overall. -Chris
--- a/mm/vmscan.c~a +++ a/mm/vmscan.c @@ -393,14 +393,15 @@ static unsigned long do_shrink_slab(stru unsigned long nr_to_scan = min(batch_size, total_scan); shrinkctl->nr_to_scan = nr_to_scan; + shrinkctl->nr_scanned = 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; - scanned += nr_to_scan; + count_vm_events(SLABS_SCANNED, shrinkctl->nr_scanned); + total_scan -= shrinkctl->nr_scanned; + scanned += shrinkctl->nr_scanned; cond_resched(); }