linux-kernel - Re: [PATCH] mm/vmalloc: Eliminate an extra orig_gfp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20211104111414.GA1928@pc638.lan>
Date:   Thu, 4 Nov 2021 12:14:14 +0100
From:   Uladzislau Rezki <urezki@...il.com>
To:     Michal Hocko <mhocko@...e.com>
Cc:     "Uladzislau Rezki (Sony)" <urezki@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        LKML <linux-kernel@...r.kernel.org>,
        Mel Gorman <mgorman@...e.de>,
        Christoph Hellwig <hch@...radead.org>,
        Matthew Wilcox <willy@...radead.org>,
        Nicholas Piggin <npiggin@...il.com>,
        Hillf Danton <hdanton@...a.com>,
        Oleksiy Avramchenko <oleksiy.avramchenko@...ymobile.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Vasily Averin <vvs@...tuozzo.com>
Subject: Re: [PATCH] mm/vmalloc: Eliminate an extra orig_gfp_mask

> [Cc Vasily]
> 
> On Wed 03-11-21 21:07:03, Uladzislau Rezki wrote:
> > That extra variable has been introduced just for keeping an original
> > passed gfp_mask because it is updated with __GFP_NOWARN on entry, thus
> > error handling messages were broken.
> 
> I am not sure what you mean by "error handling messages were broken"
> part.
> 
We slightly discussed it in another thread :) There was __GFP_NOWARN added
on entry(unconditionally), what leads to ignoring all our internal error
messages by the warn_alloc(). I have checked the linux-next and saw that
Vasily sent a patch fixing it:

<snip>
Author: Vasily Averin <vvs@...tuozzo.com>
Date:   Thu Oct 21 15:07:26 2021 +1100

    mm/vmalloc: repair warn_alloc()s in __vmalloc_area_node()
    
    Commit f255935b9767 ("mm: cleanup the gfp_mask handling in
    __vmalloc_area_node") added __GFP_NOWARN to gfp_mask unconditionally
    however it disabled all output inside warn_alloc() call.  This patch saves
    original gfp_mask and provides it to all warn_alloc() calls.
<snip>

> It is true that the current Linus tree has a broken allocation failure
> reporting but that is not a fault of orig_gfp_mask. In fact patch which
> is fixing that "mm/vmalloc: repair warn_alloc()s in
> __vmalloc_area_node()" currently in akpm tree is adding the additional
> mask.
>  
> > Instead we can keep an original gfp_mask without modifying it and add
> > an extra __GFP_NOWARN flag together with gfp_mask as a parameter to
> > the vm_area_alloc_pages() function. It will make it less confused.
> 
> I would tend to agree that this is a better approach. There is already
> nested_gfp mask and one more doesn't add to the readbility.
> 
Agree, that is why i decided to send a patch. Because i find that extra
gfp variable as odd one and confusing. I paid an attention on it during
our discussion about __GFP_NOFAIL. But on my tree it was not fixed at all
and after checking the linux-next i saw a fix.

>
> Maybe we should just drop the above patch and just go with one which
> doesn't introduce the intermediate step and an additional gfp mask.
> 
That we can do if all agree on. 

Thanks!

--
Vlad Rezki