From: Vlastimil Babka <vbabka@suse.cz>
To: Dave Chinner <david@fromorbit.com>
Cc: Christoph Hellwig <hch@lst.de>,
akpm@linux-foundation.org, osalvador@suse.de, elver@google.com,
andreyknvl@gmail.com, linux-mm@kvack.org, djwong@kernel.org,
linux-xfs@vger.kernel.org
Subject: Re: [PATCH] mm,page_owner: don't remove GFP flags in add_stack_record_to_list
Date: Tue, 30 Apr 2024 07:31:25 +0200 [thread overview]
Message-ID: <1fb5b8f3-d8c7-4350-888a-ad8f4d54bc66@suse.cz> (raw)
In-Reply-To: <ZjAyIWUzDipofHFJ@dread.disaster.area>
On 4/30/24 1:49 AM, Dave Chinner wrote:
> On Mon, Apr 29, 2024 at 09:59:43AM +0200, Vlastimil Babka wrote:
>> On 4/29/24 7:47 AM, Christoph Hellwig wrote:
>> > This loses flags like GFP_NOFS and GFP_NOIO that are important to avoid
>> > deadlocks as well as GFP_NOLOCKDEP that otherwise generates lockdep false
>> > positives.
>>
>> GFP_NOFS and GFP_NOIO translate to GFP_KERNEL without __GFP_FS/__GFP_IO so I
>> don't see how this patch would have helped with those.
>> __GFP_NOLOCKDEP is likely the actual issue and stackdepot solved it like this:
>>
>> https://lore.kernel.org/linux-xfs/20240418141133.22950-1-ryabinin.a.a@gmail.com/
>>
>> So we could just do the same here.
>
> Yes, it is __GFP_NOLOCKDEP that is the issue here, but
> cargo-cult-copying of that stackdepot fix is just whack-a-mole bug
> fixing without addressing the technical debt that got us here in the
> first place. Has anyone else bothered to look to see if kmemleak has
> the same problem?
Looks like you did :)
> If anyone bothered to do an audit, they would see that
> gfp_kmemleak_mask() handles the reclaim context masks correctly.
> Further, it adds NOWARN, NOMEMALLOC and
> NORETRY, which means the debug code is silent when it fails, it
> doesn't deplete emergency reserves and doesn't bog down retrying
> forever when there are sustained low memory situations.
So we do have NOWARN here. __GFP_RETRY_MAYFAIL might have been slightly
better than __GFP_NOWARN wrt "not retrying forever" but also not giving up
too soon. If we want to be really careful about reserves, it's a question
whether to keep the | GFP_ATOMIC which translates to leaving __GFP_HIGH.
OTOH if we don't keep it, these allocations might fail too easily from an
atomic context and we could miss the debugging data.
> This also points out that the page-owner/stackdepot code that strips
> GFP_ZONEMASK is completely redundant. Doing:
>
> gfp_flags &= GFP_KERNEL|GFP_ATOMIC|__GFP_NOLOCKDEP;
>
> strips everything but __GFP_RECLAIM, __GFP_FS, __GFP_IO,
> __GFP_HIGH and __GFP_NOLOCKDEP. This already strips the zonemask
> info, so there's no need to do it explicitly.
True.
> IOWs, the right way to fix this set of problems is to lift
> gfp_kmemleak_mask() to include/linux/gfp.h and then use it across
> all these nested allocations that occur behind the public
> memory allocation API.
Agree. But arguably these quick fixes adding __GFP_NOLOCKDEP were
appropriate for the late rc phase we're in.
> I've got a patchset under test at the moment that does this....
Great! Thanks.
> -Dave.
prev parent reply other threads:[~2024-04-30 5:31 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 5:47 [PATCH] mm,page_owner: don't remove GFP flags in add_stack_record_to_list Christoph Hellwig
2024-04-29 7:59 ` Vlastimil Babka
2024-04-29 23:49 ` Dave Chinner
2024-04-30 5:31 ` Vlastimil Babka [this message]
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=1fb5b8f3-d8c7-4350-888a-ad8f4d54bc66@suse.cz \
--to=vbabka@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=andreyknvl@gmail.com \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=elver@google.com \
--cc=hch@lst.de \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
--cc=osalvador@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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).