Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <208aff10-8a32-6ab8-f03a-7f3c9d3ca0f7@intel.com>
Date:   Wed, 19 Jul 2023 09:57:41 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.11.0
Subject: Re: [RFC PATCH v2 3/3] mm: mlock: update mlock_pte_range to handle
 large folio
Content-Language: en-US
To:     Yosry Ahmed <yosryahmed@google.com>, Yu Zhao <yuzhao@google.com>
CC:     <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
        <akpm@linux-foundation.org>, <willy@infradead.org>,
        <david@redhat.com>, <ryan.roberts@arm.com>, <shy828301@gmail.com>,
        Yin Fengwei <fengwei.yin@intel.com>,
        Hugh Dickins <hughd@google.com>
References: <20230712060144.3006358-1-fengwei.yin@intel.com>
 <20230712060144.3006358-4-fengwei.yin@intel.com>
 <CAOUHufYef--8MxFettL6fOGjVx2vyZHZQU6EEaTCoW0XBvuC8Q@mail.gmail.com>
 <CAOUHufZ6=9P_=CAOQyw0xw-3q707q-1FVV09dBNDC-hpcpj2Pg@mail.gmail.com>
 <40cbc39e-5179-c2f4-3cea-0a98395aaff1@intel.com>
 <CAOUHufZHyEvU-c2O6B6stM_QVMxc22zV4Szn52myYqjdZvptUA@mail.gmail.com>
 <16844254-7248-f557-b1eb-b8b102c877a2@intel.com>
 <CAJD7tkYAkVOE2caqEj_hTmm47Kex451prBQ1wKTRUiOwnDcwNA@mail.gmail.com>
 <b995e802-1500-6930-79d0-8cc4bfe89589@intel.com>
 <CAJD7tkZtHku-kaK02MAdgaxNzr9hQkPty=cw44R_9HdTS+Pd5w@mail.gmail.com>
 <CAJD7tkZWXdHwpW5AeKqmn6TVCXm1wmKr-2RN2baRJ7c4ciTJng@mail.gmail.com>
From:   Yin Fengwei <fengwei.yin@intel.com>
In-Reply-To: <CAJD7tkZWXdHwpW5AeKqmn6TVCXm1wmKr-2RN2baRJ7c4ciTJng@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Vk9OREFmMDdHc2ZyUTQ4Z2NpVjFCNnFxTEk1dmlZQkVWcGoxS0VmN1VreDQ2?=
 =?utf-8?B?SkY3ajlIMzZuM01kbGRqTTlwQ3o3UEtuZTNQV01PNjlPL1hqZVJhTitKNERx?=
 =?utf-8?B?ZEdESWlLbDYyaEJLYTlDWS8vTUozanJSNGFuUGxacVFtbngwL1NQQVQxUUMv?=
 =?utf-8?B?Um1FTVNRVU4zM0dEcUd2UWFONFhPNW1tT0JMbnNHYnpva2JqQnJqZUpoSCtx?=
 =?utf-8?B?dTlVS2d6Y1BBVHhKN3FRWk9qTGVkeFFVWGRJb1pMbE1CMVQ0NzByVE1rU3JI?=
 =?utf-8?B?SWk3c1cybVkyQXhIbmNzNDFYSDlqaTdBZm1pTkdwNUwzNGZja3Q2aG1sUi9x?=
 =?utf-8?B?aHFNNWRWMFVLbGVUd05ZYnpsc2djQUh2aEkrV0RVWnc5ZGpTR3VsQjI2OFpQ?=
 =?utf-8?B?Wi9OZlNCQlNGT3p6RmkwcW96RUJaMko5Rkd0SHN6WXE0TC9tZ3krYkVMWC9k?=
 =?utf-8?B?MktROUthQ0ZkNU1UNk4zQWc5Skk2WEtlVzlwQk4wY2psRGZxUHlxcGNrR1Vz?=
 =?utf-8?B?U1JCQUk2Sjh3SUFpVkh5SEFiRWdLeFcreEZMRU56ajR0RnF0aFlwanNqQ2Np?=
 =?utf-8?B?SWd4b1FIZnJzSWxuM0dlcW9GM3BFbUkyaUQ2Vi9GQTlFditLMmhHSVZoeXE5?=
 =?utf-8?B?S0w2Q29UK1dEMk00bGxic0lCK3ZqQkN4VkhPNzQySFQ4bjNsMTFreHVlNHdO?=
 =?utf-8?B?WDlkZVNCMXh2Z0hDOTlQRXc2ajErWW5PVzZNaUtpQlRTMWFSTzRHTTVTZkVL?=
 =?utf-8?B?UkJkMTNHNFBsVlh3a1d5OThyb0d1WGgvRVdWWElpaHN2bnVPcENwOWY1SEZo?=
 =?utf-8?B?dElSUXV0WXRlSzJLTU9vc2t2MkpwYkJSai9NU09hN3pzV2x0NWFkT2RQME04?=
 =?utf-8?B?SjFFTTNxN09MaXl3QVdPZlFYSU9DTW9vTlFSNHQrOExJS3ZvR1JqM05RTEdG?=
 =?utf-8?B?aCs1OENDQVF2VUlUUTNtajJieXlyV2U4OVd3SUZHazZGOTJ1RmxOZTl1V1V5?=
 =?utf-8?B?ZkhVQSsyNDFCTmJLdUtYKytzd0RWalhJbGJyRVplNlg1VlQ5VDNiOUV1RW9Q?=
 =?utf-8?B?bFFCODRYSHZIQzhWcVA2UVBGNFV5bEZZNVJQSnFpLzJBdEJvUEVoYkJkYXpk?=
 =?utf-8?B?VDRrY0JSdGluM3RmbU4xVmtsNjBGV1N4UGJ5YXMrWUxBcDRvRlpHb3A1Zmg1?=
 =?utf-8?B?R0lPWGNTSmcyc2Rmb1orcGZQOEpsUmM2MFd6cXVwTk9HOTd4M1JaYkxGVjRT?=
 =?utf-8?B?QlhhTmF4cFBUN0VRZzRQSUNSNGlwTi80SmxVazFQMDl6K0NVY1pGLzNibjRV?=
 =?utf-8?B?a0dyeTI1dWgxMTZtUzJ3SUFiYzNmM09LQWNoZkg0VTMrcGhHRHdGWVFNaHo4?=
 =?utf-8?B?eUIyQVIyenlHSFpJVE1iWGliaTYyOVRUQjRTOWN6YzdxdlhKckZ0UVRmVmYr?=
 =?utf-8?B?Qkx1SkpHL2JJaEcyZm12T3BGMGJKazU5cDZCbEt5eXdNemM0Z3lteVVsaW84?=
 =?utf-8?B?QTR4RlB0dVE0cytzWWcxR0UreFJ1OTQ3SG5sNEY0TWU2THpYVGY4MG91TzBj?=
 =?utf-8?B?SzFUbjUvRDUwMjJjazgwRjFiK2orR3FLd1B2ZWQ1YTBZQjEvaytqWHI0OWNi?=
 =?utf-8?B?SzVEekpqYktsN3dkb2p4Z0R1Vmt4YnpvSG5ZQ2kzTjE2ZGppM3RadVpTWmNa?=
 =?utf-8?B?bVo4TjVIcUxzS0EyUXJORS8wRm9vSS9vd2Z5MGgrUDVaNk1hS0dSTEtUM2Q3?=
 =?utf-8?B?aHNTam4yTm5YUmxZMkxSVlJUci9FUVpWNE52WTFHcERsOUhQaFVwTlhocVMy?=
 =?utf-8?B?ay81VStDcVlEUnBiQkRhTDhNUWFNbHphUEJRbmd0ZitVNHowc3ZxVmRURHM4?=
 =?utf-8?B?Yi9HM3MvdVVqV0hsTVFxWFdGU3o0VE9HcDZwZkhvUGRUQjRqSzFhU250bGhY?=
 =?utf-8?B?YlZrN3dCamdPR0tTVE9kM2dJZkNza3lEUitUMnVLY0Fvc0x1VCtaRkxKU1pk?=
 =?utf-8?B?RytDejlSa2tqa3VuL0J4aklRRzJqelJoa1BHdENKWEVOWWtibytKa3I4Qi9r?=
 =?utf-8?B?UFd2SzZRWDZGYzdrTjFuTTBjdzBOcG5lUnFxSDVTd0c5YzFEZWxzUGplUGlJ?=
 =?utf-8?B?Qmwxc1ozMUtvMzArenhGSWdPMWxSeHRVbkdQZmI5MEZ1d0EzYS9JZ0xMMHBS?=
 =?utf-8?B?VUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c39668d7-9faf-473f-2021-08db87fb887b
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jul 2023 01:57:40.1717
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: H7dTxa536ds4ONDOh8CcaWoiz9hIGDcG+GkHI8nZcO5nW/g8eCkDe1IrC0UWzaONiMfrUIwcjdvuMNLMn7AXSw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5389
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 7/19/23 09:52, Yosry Ahmed wrote:
> On Tue, Jul 18, 2023 at 6:32 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>> On Tue, Jul 18, 2023 at 4:47 PM Yin Fengwei <fengwei.yin@intel.com> wrote:
>>>
>>>
>>> On 7/19/23 06:48, Yosry Ahmed wrote:
>>>> On Sun, Jul 16, 2023 at 6:58 PM Yin Fengwei <fengwei.yin@intel.com> wrote:
>>>>>
>>>>>
>>>>> On 7/17/23 08:35, Yu Zhao wrote:
>>>>>> On Sun, Jul 16, 2023 at 6:00 PM Yin, Fengwei <fengwei.yin@intel.com> wrote:
>>>>>>> On 7/15/2023 2:06 PM, Yu Zhao wrote:
>>>>>>>> There is a problem here that I didn't have the time to elaborate: we
>>>>>>>> can't mlock() a folio that is within the range but not fully mapped
>>>>>>>> because this folio can be on the deferred split queue. When the split
>>>>>>>> happens, those unmapped folios (not mapped by this vma but are mapped
>>>>>>>> into other vmas) will be stranded on the unevictable lru.
>>>>>>> This should be fine unless I missed something. During large folio split,
>>>>>>> the unmap_folio() will be migrate(anon)/unmap(file) folio. Folio will be
>>>>>>> munlocked in unmap_folio(). So the head/tail pages will be evictable always.
>>>>>> It's close but not entirely accurate: munlock can fail on isolated folios.
>>>>> Yes. The munlock just clear PG_mlocked bit but with PG_unevictable left.
>>>>>
>>>>> Could this also happen against normal 4K page? I mean when user try to munlock
>>>>> a normal 4K page and this 4K page is isolated. So it become unevictable page?
>>>> Looks like it can be possible. If cpu 1 is in __munlock_folio() and
>>>> cpu 2 is isolating the folio for any purpose:
>>>>
>>>> cpu1                                        cpu2
>>>>                                                 isolate folio
>>>> folio_test_clear_lru() // 0
>>>>                                                 putback folio // add
>>>> to unevictable list
>>>> folio_test_clear_mlocked()
>>> Yes. Yu showed this sequence to me in another email. I thought the putback_lru()
>>> could correct the none-mlocked but unevictable folio. But it doesn't because
>>> of this race.
>> (+Hugh Dickins for vis)
>>
>> Yu, I am not familiar with the split_folio() case, so I am not sure it
>> is the same exact race I stated above.
>>
>> Can you confirm whether or not doing folio_test_clear_mlocked() before
>> folio_test_clear_lru() would fix the race you are referring to? IIUC,
>> in this case, we make sure we clear PG_mlocked before we try to to
>> clear PG_lru. If we fail to clear it, then someone else have the folio
>> isolated after we clear PG_mlocked, so we can be sure that when they
>> put the folio back it will be correctly made evictable.
>>
>> Is my understanding correct?
> Hmm, actually this might not be enough. In folio_add_lru() we will
> call folio_batch_add_and_move(), which calls lru_add_fn() and *then*
> sets PG_lru. Since we check folio_evictable() in lru_add_fn(), the
> race can still happen:
>
>
> cpu1                              cpu2
>                                       folio_evictable() //false
> folio_test_clear_mlocked()
> folio_test_clear_lru() //false
>                                       folio_set_lru()
>
> Relying on PG_lru for synchronization might not be enough with the
> current code. We might need to revert 2262ace60713 ("mm/munlock:
> delete smp_mb() from __pagevec_lru_add_fn()").
>
> Sorry for going back and forth here, I am thinking out loud.

Yes. Currently, the order in lru_add_fn() is not correct.

I think we should move folio_test_clear_lru(folio) into

lru locked range. As the lru lock here was expected to

use for sync here. Check the comment in lru_add_fn().


Regards

Yin, Fengwei


>
>> If yes, I can add this fix to my next version of the RFC series to
>> rework mlock_count. It would be a lot more complicated with the
>> current implementation (as I stated in a previous email).
>>
>>>>
>>>> The page would be stranded on the unevictable list in this case, no?
>>>> Maybe we should only try to isolate the page (clear PG_lru) after we
>>>> possibly clear PG_mlocked? In this case if we fail to isolate we know
>>>> for sure that whoever has the page isolated will observe that
>>>> PG_mlocked is clear and correctly make the page evictable.
>>>>
>>>> This probably would be complicated with the current implementation, as
>>>> we first need to decrement mlock_count to determine if we want to
>>>> clear PG_mlocked, and to do so we need to isolate the page as
>>>> mlock_count overlays page->lru. With the proposal in [1] to rework
>>>> mlock_count, it might be much simpler as far as I can tell. I intend
>>>> to refresh this proposal soon-ish.
>>>>
>>>> [1]https://lore.kernel.org/lkml/20230618065719.1363271-1-yosryahmed@google.com/
>>>>
>>>>>
>>>>> Regards
>>>>> Yin, Fengwei
>>>>>