Re: [PATCH 1/2] mm: Add memalloc_nowait_{save,restore}

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 14, 2024 at 3:42 PM Michal Hocko <mhocko@xxxxxxxx> wrote:
>
> On Mon 12-08-24 20:59:53, Yafang Shao wrote:
> > On Mon, Aug 12, 2024 at 7:37 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> > >
> > > On Mon, Aug 12, 2024 at 05:05:24PM +0800, Yafang Shao wrote:
> > > > The PF_MEMALLOC_NORECLAIM flag was introduced in commit eab0af905bfc
> > > > ("mm: introduce PF_MEMALLOC_NORECLAIM, PF_MEMALLOC_NOWARN"). To complement
> > > > this, let's add two helper functions, memalloc_nowait_{save,restore}, which
> > > > will be useful in scenarios where we want to avoid waiting for memory
> > > > reclamation.
> > >
> > > No, forcing nowait on callee contets is just asking for trouble.
> > > Unlike NOIO or NOFS this is incompatible with NOFAIL allocations
> >
> > I don’t see any incompatibility in __alloc_pages_slowpath(). The
> > ~__GFP_DIRECT_RECLAIM flag only ensures that direct reclaim is not
> > performed, but it doesn’t prevent the allocation of pages from
> > ALLOC_MIN_RESERVE, correct?
>
> Right but this means that you just made any potential nested allocation
> within the scope that is GFP_NOFAIL a busy loop essentially. Not to
> mention it BUG_ON as non-sleeping GFP_NOFAIL allocations are
> unsupported. I believe this is what Christoph had in mind.

If that's the case, I believe we should at least consider adding the
following code change to the kernel:

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9ecf99190ea2..89411ee23c7f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4168,6 +4168,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
        unsigned int zonelist_iter_cookie;
        int reserve_flags;

+       WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL && !(gfp_mask &
__GFP_DIRECT_RECLAIM));
 restart:
        compaction_retries = 0;

        no_progress_loops = 0;


> I am really
> surprised that we even have PF_MEMALLOC_NORECLAIM in the first place!

There's use cases for it.

> Unsurprisingly this was merged without any review by the MM community :/
> --
> Michal Hocko
> SUSE Labs

--
Regards
Yafang





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux