linux-kernel - Re: [PATCH 2/4] mm/vmalloc: add support for __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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YXwI7+1bQNECvBz4@dhcp22.suse.cz>
Date:   Fri, 29 Oct 2021 16:45:03 +0200
From:   Michal Hocko <mhocko@...e.com>
To:     Uladzislau Rezki <urezki@...il.com>
Cc:     Linux Memory Management List <linux-mm@...ck.org>,
        Dave Chinner <david@...morbit.com>, Neil Brown <neilb@...e.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Hellwig <hch@...radead.org>,
        linux-fsdevel@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        Ilya Dryomov <idryomov@...il.com>,
        Jeff Layton <jlayton@...nel.org>
Subject: Re: [PATCH 2/4] mm/vmalloc: add support for __GFP_NOFAIL

On Fri 29-10-21 16:05:32, Uladzislau Rezki wrote:
[...]
> > OK, this looks easier from the code reading but isn't it quite wasteful
> > to throw all the pages backing the area (all of them allocated as
> > __GFP_NOFAIL) just to then fail to allocate few page tables pages and
> > drop all of that on the floor (this will happen in __vunmap AFAICS).
> >
> > I mean I do not care all that strongly but it seems to me that more
> > changes would need to be done here and optimizations can be done on top.
> >
> > Is this something you feel strongly about?
> >
> Will try to provide some motivations :)
> 
> It depends on how to look at it. My view is as follows a more simple code
> is preferred. It is not considered as a hot path and it is rather a corner
> case to me.

Yes, we are definitely talking about corner cases here. Even GFP_KERNEL
allocations usually do not fail.

> I think "unwinding" has some advantage. At least one motivation
> is to release a memory(on failure) before a delay that will prevent holding
> of extra memory in case of __GFP_NOFAIL infinitelly does not succeed, i.e.
> if a process stuck due to __GFP_NOFAIL it does not "hold" an extra memory
> forever.

Well, I suspect this is something that we can disagree on and both of us
would be kinda right. I would see it as throwing baby out with the
bathwater. The vast majority of the memory will be in the area pages and
sacrificing that just to allocate few page tables or whatever that might
fail in that code path is just a lot of cycles wasted.

So unless you really feel strongly about this then I would stick with
this approach.
-- 
Michal Hocko
SUSE Labs