On Tue, Jan 31, 2017 at 5:26 AM, Minchan Kim <minc...@kernel.org> wrote:
> On Fri, Jan 27, 2017 at 01:43:36PM +0530, Vinayak Menon wrote:
>> It is noticed that during a global reclaim the memory
>> reclaimed via shrinking the slabs can sometimes result
>> in reclaimed pages being greater than the scanned pages
>> in shrink_node. When this is passed to vmpressure, the
>> unsigned arithmetic results in the pressure value to be
>> huge, thus resulting in a critical event being sent to
>> root cgroup. While this can be fixed by underflow checks
>> in vmpressure, adding reclaimed slab without a corresponding
>> increment of nr_scanned results in incorrect vmpressure
>> reporting. So do not consider reclaimed slab pages in
>> vmpressure calculation.
>
> I belive we could enhance the description better.
>
> problem
>
> VM include nr_reclaimed of slab but not nr_scanned so pressure
> calculation can be underflow.
>
> solution
>
> do not consider reclaimed slab pages for vmpressure
>
> why
>
> Freeing a page by slab shrinking depends on each slab's object
> population so the cost model(i.e., scan:free) is not fair with
> LRU pages. Also, every shrinker doesn't account reclaimed pages.
> Lastly, this regression happens since 6b4f7799c6a5
>
Done. Sending an updated one.
>>
>> Signed-off-by: Vinayak Menon <vinme...@codeaurora.org>
>> ---
>> mm/vmscan.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 947ab6f..37c4486 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -2594,16 +2594,16 @@ static bool shrink_node(pg_data_t *pgdat, struct
>> scan_control *sc)
>> sc->nr_scanned - nr_scanned,
>> node_lru_pages);
>>
>> - if (reclaim_state) {
>> - sc->nr_reclaimed += reclaim_state->reclaimed_slab;
>> - reclaim_state->reclaimed_slab = 0;
>> - }
>> -
>> /* Record the subtree's reclaim efficiency */
>> vmpressure(sc->gfp_mask, sc->target_mem_cgroup, true,
>> sc->nr_scanned - nr_scanned,
>> sc->nr_reclaimed - nr_reclaimed);
>>
>
> Please add comment about "vmpressure excludes reclaimed pages via slab
> because blah blah blah" so upcoming patches doesn't make mistake again.
>
> Thanks!
>
Done. Thanks Minchan.
