From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qg0-f52.google.com (mail-qg0-f52.google.com [209.85.192.52]) by kanga.kvack.org (Postfix) with ESMTP id 7778B6B0038 for ; Thu, 10 Sep 2015 04:48:28 -0400 (EDT) Received: by qgez77 with SMTP id z77so29678484qge.1 for ; Thu, 10 Sep 2015 01:48:28 -0700 (PDT) Received: from mail-qg0-x22e.google.com (mail-qg0-x22e.google.com. [2607:f8b0:400d:c04::22e]) by mx.google.com with ESMTPS id w184si12436871qkw.97.2015.09.10.01.48.27 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 10 Sep 2015 01:48:27 -0700 (PDT) Received: by qgt47 with SMTP id 47so29499415qgt.2 for ; Thu, 10 Sep 2015 01:48:27 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <55F1259C.3020006@suse.cz> References: <55F072EA.4000703@redhat.com> <55F1259C.3020006@suse.cz> Date: Thu, 10 Sep 2015 10:48:26 +0200 Message-ID: Subject: Re: [PATCH/RFC] mm: do not regard CMA pages as free on watermark check From: Vitaly Wool Content-Type: multipart/alternative; boundary=001a1135c678a90672051f60a9c2 Sender: owner-linux-mm@kvack.org List-ID: To: Vlastimil Babka Cc: Laura Abbott , Vitaly Wool , LKML , "linux-mm@kvack.org" , Joonsoo Kim , Mel Gorman --001a1135c678a90672051f60a9c2 Content-Type: text/plain; charset=UTF-8 On Thu, Sep 10, 2015 at 8:39 AM, Vlastimil Babka wrote: > [CC Joonsoo, Mel] > > On 09/09/2015 08:31 PM, Vitaly Wool wrote: > > Hi Laura, > > > > On Wed, Sep 9, 2015 at 7:56 PM, Laura Abbott wrote: > > > >> (cc-ing linux-mm) > >> On 09/09/2015 07:44 AM, Vitaly Wool wrote: > >> > >>> __zone_watermark_ok() does not corrrectly take high-order > >>> CMA pageblocks into account: high-order CMA blocks are not > >>> removed from the watermark check. Moreover, CMA pageblocks > >>> may suddenly vanish through CMA allocation, so let's not > >>> regard these pages as free in __zone_watermark_ok(). > >>> > >>> This patch also adds some primitive testing for the method > >>> implemented which has proven that it works as it should. > >>> > >>> > >> The choice to include CMA as part of watermarks was pretty deliberate. > >> Do you have a description of the problem you are facing with > >> the watermark code as is? Any performance numbers? > >> > >> > > let's start with facing the fact that the calculation in > > __zone_watermark_ok() is done incorrectly for the case when ALLOC_CMA is > > not set. While going through pages by order it is implicitly considered > > You're not the first who tried to fix it, I think Joonsoo tried as well? > I think the main objection was against further polluting fastpaths due to > CMA. > I believe Joonsoo was calculating free_pages incorrectly, too, but in a different way: he was subtracting CMA pages twice. > Note that Mel has a patchset removing high-order watermark checks (in the > last > patch of https://lwn.net/Articles/655406/ ) so this will be moot > afterwards. > I am not quite convinced that nested loops are a better solution than what I suggest. > > > that CMA pages can be used and this impacts the result of the function. > > > > This can be solved in a slightly different way compared to what I > proposed > > but it needs per-order CMA pages accounting anyway. Then it would have > > looked like: > > > > for (o = 0; o < order; o++) { > > /* At the next order, this order's pages become > unavailable > > */ > > free_pages -= z->free_area[o].nr_free << o; > > #ifdef CONFIG_CMA > > if (!(alloc_flags & ALLOC_CMA)) > > free_pages -= z->free_area[o].nr_free_cma << o; > > /* Require fewer higher order pages to be free */ > > min >>= 1; > > ... > > > > But what we have also seen is that CMA pages may suddenly disappear due > to > > CMA allocator work so the whole watermark checking was still unreliable, > > causing compaction to not run when it ought to and thus leading to > > Well, watermark checking is inherently racy. CMA pages disappearing is no > exception, non-CMA pages may disappear as well. > Right, that is why I decided to play on the safe side. ~vitaly --001a1135c678a90672051f60a9c2 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Thu, Sep 10, 2015 at 8:39 AM, Vlastimil Babka <= vbabka@suse.cz><= /span> wrote:
[CC Joonsoo, Mel]

On 09/09/2015 08:31 PM, Vitaly Wool wrote:
> Hi Laura,
>
> On Wed, Sep 9, 2015 at 7:56 PM, Laura Abbott <labbott@redhat.com> wrote:
>
>> (cc-ing linux-mm)
>> On 09/09/2015 07:44 AM, Vitaly Wool wrote:
>>
>>> __zone_watermark_ok() does not corrrectly take high-order
>>> CMA pageblocks into account: high-order CMA blocks are not
>>> removed from the watermark check. Moreover, CMA pageblocks
>>> may suddenly vanish through CMA allocation, so let's not >>> regard these pages as free in __zone_watermark_ok().
>>>
>>> This patch also adds some primitive testing for the method
>>> implemented which has proven that it works as it should.
>>>
>>>
>> The choice to include CMA as part of watermarks was pretty deliber= ate.
>> Do you have a description of the problem you are facing with
>> the watermark code as is? Any performance numbers?
>>
>>
> let's start with facing the fact that the calculation in
> __zone_watermark_ok() is done incorrectly for the case when ALLOC_CMA = is
> not set. While going through pages by order it is implicitly considere= d

You're not the first who tried to fix it, I think Joonsoo tried = as well?
I think the main objection was against further polluting fastpaths due to C= MA.

I believe Joonsoo was calculating f= ree_pages incorrectly, too, but in a different way: he was subtracting CMA = pages twice.


Note that Mel has a patchset removing high-order watermark checks (in the l= ast
patch of https://lwn.net/Articles/655406/ ) so this will be moot= afterwards.

I am not quite convinced t= hat nested loops are a better solution than what I suggest.
=C2= =A0

> that CMA pages can be used and this impacts the result of the function= .
>
> This can be solved in a slightly different way compared to what I prop= osed
> but it needs per-order CMA pages accounting anyway. Then it would have=
> looked like:
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0for (o =3D 0; o < order; o++) { >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* At the= next order, this order's pages become unavailable
> */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0free_page= s -=3D z->free_area[o].nr_free << o;
> #ifdef CONFIG_CMA
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!(all= oc_flags & ALLOC_CMA))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0free_pages -=3D z->free_area[o].nr_free_cma <<= o;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Requir= e fewer higher order pages to be free */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0min >&= gt;=3D 1;
> ...
>
> But what we have also seen is that CMA pages may suddenly disappear du= e to
> CMA allocator work so the whole watermark checking was still unreliabl= e,
> causing compaction to not run when it ought to and thus leading to

Well, watermark checking is inherently racy. CMA pages disappearing = is no
exception, non-CMA pages may disappear as well.

Right, that is why I decided to play on the safe side.
<= br>
=C2=A0~vitaly
--001a1135c678a90672051f60a9c2-- -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org