From: Uladzislau Rezki <urezki@gmail.com>
To: Michal Hocko <mhocko@suse.com>
Cc: Uladzislau Rezki <urezki@gmail.com>,
Linux Memory Management List <linux-mm@kvack.org>,
Dave Chinner <david@fromorbit.com>, Neil Brown <neilb@suse.de>,
Andrew Morton <akpm@linux-foundation.org>,
Christoph Hellwig <hch@infradead.org>,
linux-fsdevel@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
Ilya Dryomov <idryomov@gmail.com>,
Jeff Layton <jlayton@kernel.org>
Subject: Re: [PATCH 2/4] mm/vmalloc: add support for __GFP_NOFAIL
Date: Fri, 29 Oct 2021 19:23:07 +0200 [thread overview]
Message-ID: <20211029172307.GA2944@pc638.lan> (raw)
In-Reply-To: <YXwI7+1bQNECvBz4@dhcp22.suse.cz>
> 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.
>
We are not talking about performance, no sense to measure cycles here :)
>
> So unless you really feel strongly about this then I would stick with
> this approach.
>
I have raised one concern. The memory resource is shared between all
process in case of __GFP_NOFAIL it might be that we never return back
to user in that scenario i prefer to release hold memory for other
needs instead of keeping it for nothing.
If you think it is not a problem, then i do not have much to say.
--
Vlad Rezki
next prev parent reply other threads:[~2021-10-29 17:23 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-25 15:02 [PATCH 0/4] extend vmalloc support for constrained allocations Michal Hocko
2021-10-25 15:02 ` [PATCH 1/4] mm/vmalloc: alloc GFP_NO{FS,IO} for vmalloc Michal Hocko
2021-10-25 15:02 ` [PATCH 2/4] mm/vmalloc: add support for __GFP_NOFAIL Michal Hocko
2021-10-25 22:59 ` NeilBrown
2021-10-26 7:03 ` Michal Hocko
2021-10-26 10:30 ` NeilBrown
2021-10-26 11:29 ` Michal Hocko
2021-10-26 15:48 ` Uladzislau Rezki
2021-10-26 16:28 ` Michal Hocko
2021-10-26 19:33 ` Uladzislau Rezki
2021-10-27 6:46 ` Michal Hocko
2021-10-27 17:55 ` Uladzislau Rezki
2021-10-29 7:57 ` Michal Hocko
2021-10-29 14:05 ` Uladzislau Rezki
2021-10-29 14:45 ` Michal Hocko
2021-10-29 17:23 ` Uladzislau Rezki [this message]
2021-10-25 15:02 ` [PATCH 3/4] mm/vmalloc: be more explicit about supported gfp flags Michal Hocko
2021-10-25 23:26 ` NeilBrown
2021-10-26 7:10 ` Michal Hocko
2021-10-26 10:43 ` NeilBrown
2021-10-26 12:20 ` Michal Hocko
2021-10-25 15:02 ` [PATCH 4/4] mm: allow !GFP_KERNEL allocations for kvmalloc Michal Hocko
2021-10-25 23:34 ` NeilBrown
2021-10-26 7:15 ` Michal Hocko
2021-10-26 10:48 ` NeilBrown
2021-10-26 12:23 ` Michal Hocko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211029172307.GA2944@pc638.lan \
--to=urezki@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=david@fromorbit.com \
--cc=hch@infradead.org \
--cc=idryomov@gmail.com \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=neilb@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox