From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 33FFFC433EF for ; Tue, 3 May 2022 18:08:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8C6E36B00B5; Tue, 3 May 2022 14:08:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 854C66B00B6; Tue, 3 May 2022 14:08:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6A2696B00B7; Tue, 3 May 2022 14:08:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.26]) by kanga.kvack.org (Postfix) with ESMTP id 584A96B00B5 for ; Tue, 3 May 2022 14:08:29 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 43D62608CB for ; Tue, 3 May 2022 18:08:29 +0000 (UTC) X-FDA: 79425216738.22.6C118B0 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by imf19.hostedemail.com (Postfix) with ESMTP id C7B241A007F for ; Tue, 3 May 2022 18:08:22 +0000 (UTC) Received: by mail-pl1-f173.google.com with SMTP id s14so15654802plk.8 for ; Tue, 03 May 2022 11:08:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=TttXqLUVM2RUYcddmpbzTO7OqJ9zNGbHPbTMQOyiNYg=; b=ekJaSHYhcsRmVQABuTaZ47dOTUN9IgcSoi9eRGTW2HnBfftv2scK1Zn6X+TrUXW01C 1IBEHbKciriXj6hlLOAMMLNB7e8205lSTnbb+lVpAAjLR9Qq74WxNEYpcFXpVI7nqXIa 0xG1fOTj7+tpDRHvxKX7dy361g+gRJpEk2GeYeFU3am+0+MIcnT+5TaVus6D/7qDgzS7 rGanXqJy7MhRwVU3LaxijcSYkLozjAIzaMkCnCxBBtk1Zd5bKQMVnLFRYKr+YtxGneQy d251Hsi+ICvSjFyqsWvbQ9kkx83gSTww87m+o5mVr5BZ8DxhNUNyek9NCnppgvkab49N GXxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=TttXqLUVM2RUYcddmpbzTO7OqJ9zNGbHPbTMQOyiNYg=; b=qEFYlqYXB4gf4AA5I3Ma0MId7WGcWSmWXt43kQg4gTxf6hFX6/t1cs3pzPmRS8n1Fv sl/v2MjdODnkf4EGMgRRxSseAgaYPCoyF5qBVevdhZ4u96b+iBRfefPzvdwn+GLapHG/ pAlxcp2HUruG9FTuE8pgaH1lg0oeeUCskHR08fts5Zr0C4sTyML00hrKXgWM1quEHQ/0 jrw2xYhKJv1uryPpaFIuU+PUDWCxctVWRKPpmsRMAoYxZlJTmW8yphH1BYEY7dvwyN+w MNr3XS6GsUMWwK8q5ZRQUt0RFOgsxSc8gwr95mfk4j5Bm7UP414rATzbO8oGxNPKDFJP N8Mw== X-Gm-Message-State: AOAM533O1MnToBO871IWgzDlh30YPe2mxxurr60n6rHFfBWGm7Me1f5b x/yrgnjJfLKyiLUV5WyfSuE= X-Google-Smtp-Source: ABdhPJydu4K9sL6j3Nm0fG7o8gS9ZV86d0mg8BCm5EO9+gPM3A5XHTLmCJZxSM/8Zy0jHkWpvvWOXg== X-Received: by 2002:a17:90a:4f0b:b0:1d9:acbd:1204 with SMTP id p11-20020a17090a4f0b00b001d9acbd1204mr6031920pjh.201.1651601307400; Tue, 03 May 2022 11:08:27 -0700 (PDT) Received: from google.com ([2620:15c:211:201:8998:54e:9def:1e7c]) by smtp.gmail.com with ESMTPSA id w3-20020a17090a8a0300b001cd4989ff40sm1655162pjn.7.2022.05.03.11.08.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 May 2022 11:08:26 -0700 (PDT) Date: Tue, 3 May 2022 11:08:25 -0700 From: Minchan Kim To: David Hildenbrand Cc: Andrew Morton , linux-mm , LKML , John Hubbard , John Dias Subject: Re: [PATCH] mm: fix is_pinnable_page against on cma page Message-ID: References: <20220502173558.2510641-1-minchan@kernel.org> <29d0c1c3-a44e-4573-7e7e-32be07544dbe@redhat.com> <08e9855c-395d-f40c-de3d-1ec8b644bfe8@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: C7B241A007F X-Stat-Signature: g5sjpi57zhoucwn4wx7f4xzrmxtreu3o X-Rspam-User: Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ekJaSHYh; spf=pass (imf19.hostedemail.com: domain of minchan.kim@gmail.com designates 209.85.214.173 as permitted sender) smtp.mailfrom=minchan.kim@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) X-HE-Tag: 1651601302-976997 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, May 03, 2022 at 07:27:06PM +0200, David Hildenbrand wrote: > >> GUP would see MIGRATE_ISOLATE and would reject pinning. The page has to > >> be migrated, which can fail if the page is temporarily unmovable. > > > > Why is the page temporarily unmovable? The GUP didn't increase the > > refcount in the case. If it's not migrabtable, that's not a fault > > from the GUP but someone is already holding the temporal refcount. > > It's not the scope this patchset would try to solve it. > > You can have other references on the page that turn it temporarily > unmovable, for example, via FOLL_GET, short-term FOLL_PIN. Sure. However, user didn't passed the FOLL_LONGTERM. In that case, the temporal page migration failure was expected. What we want to guarantee for successful page migration is only FOLL_LONGTERM. If you are talking about the general problem(any GUP API without FOLL_LONGTERM flag which is supposed to be short-term could turn into long-term pinning by several reasons - I had struggled with those issues - FOLL_LONGTERM is misnormer to me), yeah, I agree we need to fix it but it's orthgonal issue. > > > > >> > >> See my point? We will try migrating in cases where we don't have to > > > > Still not clear for me what you are concerning. > > > >> migrate. I think what we would want to do is always reject pinning a CMA > >> page, independent of the isolation status. but we don't have that > > > > Always reject pinning a CMA page if it is *FOLL_LONGTERM* > > Yes. > > > > >> information available. > > > > page && (MIGRATE_CMA | MIGRATE_ISOLATE) && gup_flags is not enough > > for it? > > > >> > >> I raised in the past that we should look into preserving the migration > >> type and turning MIGRATE_ISOLATE essentially into an additional flag. > >> > >> > >> So I guess this patch is the right thing to do for now, but I wanted to > >> spell out the implications. > > > > I want but still don't understand what you want to write further > > about the implication parts. If you make more clear, I am happy to > > include it. > > What I am essentially saying is that when rejecting to long-term > FOLL_PIN something that is MIGRATE_ISOLATE now, we might now end up > having to migrate pages that are actually fine to get pinned, because > they are not actual CMA pages. And any such migration might fail when > pages are temporarily unmovable. Now I understand concern. Then how about introducing cma areas list and use it instead of migrate type in is_pinnable_page struct cma { .. .. list_head list }; bool is_cma_page(unsigned long pfn) { for cma in cma_list if (pfn >= cma->base_pfn && pfn < cma->base_pfn + count return true; return false; } Do you want to fix it at this moment or just write down the possibility in the description and then we could fix once it really happens later? > > > > > >> > >>> > >>> A thing to get some attention is whether we need READ_ONCE or not > >>> for the local variable mt. > >>> > >> > >> Hmm good point. Staring at __get_pfnblock_flags_mask(), I don't think > >> there is anything stopping the compiler from re-reading the value. But > >> we don't care if we're reading MIGRATE_CMA or MIGRATE_ISOLATE, not > >> something in between. > > > > How about this? > > > > CPU A CPU B > > > > is_pinnable_page > > .. > > .. set_pageblock_migratetype(MIGRATE_ISOLATE) > > mt == MIGRATE_CMA > > get_pageblock_miratetype(page) > > returns MIGRATE_ISOLATE > > mt == MIGRATE_ISOLATE set_pageblock_migratetype(MIGRATE_CMA) > > get_pageblock_miratetype(page) > > returns MIGRATE_CMA > > > > So both conditions fails to detect it. > > I think you're right. That's nasty. Ccing Paul to borrow expertise. :) int mt = get_pageblock_migratetype(page); if (mt == MIGRATE_CMA) return true; if (mt == MIGRATE_ISOLATE) return true; I'd like to keep use the local variable mt's value in folloing conditions checks instead of refetching the value from get_pageblock_migratetype. What's the right way to achieve it? Thanks in advance!