Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 17 Jul 2020 13:08:06 +0200
From:   Michal Hocko <mhocko@kernel.org>
To:     Joonsoo Kim <js1304@gmail.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Linux Memory Management List <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>, kernel-team@lge.com,
        Vlastimil Babka <vbabka@suse.cz>,
        Christoph Hellwig <hch@infradead.org>,
        Roman Gushchin <guro@fb.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: Re: [PATCH 2/4] mm/gup: restrict CMA region by using allocation
 scope API
Message-ID: <20200717110806.GI10655@dhcp22.suse.cz>
References: <1594789529-6206-1-git-send-email-iamjoonsoo.kim@lge.com>
 <1594789529-6206-2-git-send-email-iamjoonsoo.kim@lge.com>
 <20200715082401.GC5451@dhcp22.suse.cz>
 <CAAmzW4P+KXn2e1pU+_+Y6NqDt1-081hCyKNsqbiwfF=FADLzxQ@mail.gmail.com>
 <20200717082643.GC10655@dhcp22.suse.cz>
 <CAAmzW4N6mQ_9jrUN5NUURpxa7tf4nKwsrgiWe79v0vNJO0_6Xg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAAmzW4N6mQ_9jrUN5NUURpxa7tf4nKwsrgiWe79v0vNJO0_6Xg@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri 17-07-20 18:28:16, Joonsoo Kim wrote:
> 2020년 7월 17일 (금) 오후 5:26, Michal Hocko <mhocko@kernel.org>님이 작성:
> >
> > On Fri 17-07-20 16:46:38, Joonsoo Kim wrote:
> > > 2020년 7월 15일 (수) 오후 5:24, Michal Hocko <mhocko@kernel.org>님이 작성:
> > > >
> > > > On Wed 15-07-20 14:05:27, Joonsoo Kim wrote:
> > > > > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> > > > >
> > > > > We have well defined scope API to exclude CMA region.
> > > > > Use it rather than manipulating gfp_mask manually. With this change,
> > > > > we can now use __GFP_MOVABLE for gfp_mask and the ZONE_MOVABLE is also
> > > > > searched by page allocator. For hugetlb, gfp_mask is redefined since
> > > > > it has a regular allocation mask filter for migration target.
> > > > >
> > > > > Note that this can be considered as a fix for the commit 9a4e9f3b2d73
> > > > > ("mm: update get_user_pages_longterm to migrate pages allocated from
> > > > > CMA region"). However, "Fixes" tag isn't added here since it is just
> > > > > suboptimal but it doesn't cause any problem.
> > > >
> > > > But it is breaking the contract that the longterm pins never end up in a
> > > > cma managed memory. So I think Fixes tag is really due. I am not sure
> > > > about stable backport. If the patch was the trivial move of
> > >
> > > Previous implementation is correct since longterm pins never end up in a CMA
> > > managed memory with that implementation. It's just a different and suboptimal
> > > implementation to exclude the CMA area. This is why I don't add the "Fixes"A
> > > tag on the patch.
> >
> > But the current implementation calls memalloc_nocma_restore too early so
> > __gu_longterm_locked will migrate pages possibly to CMA ranges as there
> > is no GFP_MOVABLE restriction in place. Or am I missing something?
> 
> IIUC, calling memalloc_nocma_restore() too early doesn't cause the
> actual problem.
> 
> Final check is done by check_and_migrate_cma_pages() which is outside of
> scope API. If we find a CMA page in between the gup range here, the page is
> migrated to the migration target page and this target page is allocated by
> new_non_cma_page().
> 
> new_non_cma_page() try to allocate the page without __GFP_MOVABLE so
> returned page will not be CMA area memory. Therefore, even if
> memalloc_nocma_restore() is called early, there is no actual problem.

Right you are! I have misread gfp_t gfp_mask = GFP_USER | __GFP_NOWARN
and didn't realize that it doesn't include MOVABLE flag.

Sorry about the noise! The issue is only formal coding style so Fixes
tag could indeed cause more confusion than it would help.

-- 
Michal Hocko
SUSE Labs