[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOaiJ-mut9NO_+bj28DAz-yXbcUocvMjPVx=t=2umE+5Fp2kYQ@mail.gmail.com>
Date: Tue, 31 Jan 2017 13:18:40 +0530
From: vinayak menon <vinayakm.list@...il.com>
To: Minchan Kim <minchan@...nel.org>
Cc: Vinayak Menon <vinmenon@...eaurora.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Johannes Weiner <hannes@...xchg.org>,
mgorman@...hsingularity.net, vbabka@...e.cz, mhocko@...e.com,
Rik van Riel <riel@...hat.com>, vdavydov.dev@...il.com,
anton.vorontsov@...aro.org, shashim@...eaurora.org,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2 v2] mm: vmscan: do not pass reclaimed slab to vmpressure
On Tue, Jan 31, 2017 at 5:26 AM, Minchan Kim <minchan@...nel.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 <vinmenon@...eaurora.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.
Powered by blists - more mailing lists