Received-SPF: pass (google.com: domain of linux-kernel+bounces-101042-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <b5017375-d43b-44f9-b931-3046ebead9cf@intel.com>
Date: Wed, 13 Mar 2024 10:07:06 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] mm/migrate: put dest folio on deferred split list if
 source was there.
Content-Language: en-US
To: Matthew Wilcox <willy@infradead.org>, Zi Yan <ziy@nvidia.com>
CC: <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>, Yang Shi
	<shy828301@gmail.com>, Huang Ying <ying.huang@intel.com>, "Kirill A .
 Shutemov" <kirill.shutemov@linux.intel.com>, Ryan Roberts
	<ryan.roberts@arm.com>, <linux-kernel@vger.kernel.org>
References: <20240311195848.135067-1-zi.yan@sent.com>
 <Ze_P6xagdTbcu1Kz@casper.infradead.org>
 <74AF044A-A14A-4C66-A019-70F8F138A9AB@nvidia.com>
 <ZfBkjf4z-jpaNk6c@casper.infradead.org>
 <DFA2B8B7-C690-422A-BE95-82F7E112BB95@nvidia.com>
 <ZfCFCGkrLVpySa6X@casper.infradead.org>
 <76BBDB5D-9D40-44EB-A996-767404B85BE0@nvidia.com>
 <ZfCi7kvLoBvUnARz@casper.infradead.org>
From: "Yin, Fengwei" <fengwei.yin@intel.com>
In-Reply-To: <ZfCi7kvLoBvUnARz@casper.infradead.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ajFSekZmR1g4UmlWSDBHN0xYdXVhUE9UQUVMOUphVW9TQWxCdHJTanF4elJu?=
 =?utf-8?B?T0puMW14L0JZNkZ0WDN5c0ZIQjdCMFBpQ0VaelRGd2QvazByZVBrQlpSczdu?=
 =?utf-8?B?QzdRNEI4THhrZmFKOW1NaUhjaUNVWXRaMVZXSXM3K001aUpPR0FMT1k5ZFRq?=
 =?utf-8?B?WGE3OC9XRDRNRzJuVUh5bVZiZkZVMUZyallnTGl5Rmk0TzRTTUx3MzU2TXVa?=
 =?utf-8?B?emZ1YytxSUU5Z0tvNFNYM3kwdEFDTW01ZkxDVEJXZ3NTV3VlTmJjdEtGZURL?=
 =?utf-8?B?UzJZK0tBaVZDMG96RDU5WmZuQkFLd3B6Y1ZBL3Urbi9oQXlzQmt0YzE0MVh1?=
 =?utf-8?B?cDFaZDJHczJzNlN4ZHY1SkdhS2UrZ3g3UWpmVTF2ZElNT1BGS3pUbnpHSVpY?=
 =?utf-8?B?bCtpaUh1NkdKUTNNRmJRRituVGFtUUxZc0pPVlJTcjQrWERRbGVuL2x2b3FP?=
 =?utf-8?B?L25IVzRKeVZaS2p4eXB3dHFyTlR6V2QrRDk1VzdDM0xGMTJFWGd3K2NncWdy?=
 =?utf-8?B?WlJzeWo4Tm9ZeVZsNGIyVXVtb3RwdHhDenp6SE1KTldudTFGU09LTzR4emlo?=
 =?utf-8?B?VDY3UFZMM0xNbGxLNXdSRkhJNVlEL3RLMkJlb0QrWk9kaVR4TnlkN084RUVO?=
 =?utf-8?B?Nmp6cTR2MTdLcWRRNUkrQnVYWjNtQmZRbHkvNFpIWTU0aFlTSElGSmJGZ1p5?=
 =?utf-8?B?QkM4bDhsU1FrQUhSaHlPSEZlY3E1RC9CNERIY0p3cndIZkZMT0xySjVtRkR5?=
 =?utf-8?B?RW5LRkhNRmlnbk83YmFRSlYxdHR5ZWlBTnpveE9sUXRTc242NXhDVzJja2hV?=
 =?utf-8?B?STA5cDYrT3lnVnR1aEN4YTFDRkM1VEtVazg1SDVWYjdtUjdyNmxmbWZ6RWNx?=
 =?utf-8?B?V1pCYUFVa0REalFWakxSRENqLzlZTDdtVXNjc29lVmw1dkI3akFCTGt6Z25S?=
 =?utf-8?B?YlN3Y21TSnVjdm5FakRlT0hQeFdJMlJzQmNLNFdobmcyejBhNHVJNnhPY1l0?=
 =?utf-8?B?MWVXelg1anRtSkMyYUVDODVZOFFQNHNoNjZYQjNYTzJjRE1zODA4a1Q1Ynlh?=
 =?utf-8?B?c21JR1pZaHNDWWU3Sy9HR21EcG5ETTN6dWlpTHo3YkdmWGhxMEg3bjRHbVox?=
 =?utf-8?B?MjF5RGcyVFFzck4yR1JNenE4QTV0MkYvWHAwOE94T1FaeWdyZ3hadWRNWVJT?=
 =?utf-8?B?OTVYMjNmSUJzVEdwNy95UFUxRFJScmd3KzhtZVpFeUE3VmxldWw2MEhibHhH?=
 =?utf-8?B?SWJLUkkxb0x5K1kyS1lDR3NlUEVSeXNVWUdUZTd1Tmc3clg0b2dnOVFDQ0dC?=
 =?utf-8?B?MWI1Sk91SHJ6S1lvNEl6UzFpYWFRNkdKb1FkSnNzSzF5SHR4b3hENWE2bzRa?=
 =?utf-8?B?RVoyZHAyMTFRQklXWW9yL1dLZ1d5LzVObDZaMlg3Q1ZMeEdVVTZsUGRUVU5K?=
 =?utf-8?B?a0VkMHZTeVJEYzN5MDBsZXBEMXZ2dzg5L21idDlZMFI3dlh1YjB6MmdwMG9H?=
 =?utf-8?B?RTB3STNkS2crdGNOTDVnZWkwTkpwZW1xSnZQT1FKSnBranhXY3g4NTA3SUg4?=
 =?utf-8?B?d20xVHZQekxPSWo2c2ltQkJnYmRUOW1Oa0hqNEhXem8yUG4zSzdVeW9wbS90?=
 =?utf-8?B?TXdGWTlGelB2OUpJZm8rOFpjZTFtdU5QeGVHSjdzbDBBclVwS1hBOTNDOUoy?=
 =?utf-8?B?SFk4TXhKYi9YZERqWG5iRXkxQzBrckVvQW43TnpOZ1Zqc2d4eGR0eUJrcDI3?=
 =?utf-8?B?cnVWR3l1cVROWkZLdE1yWUtiTWtuS1d2Z0ZlVFZIOUpkSEgySmFCcENxRzVt?=
 =?utf-8?B?RjdJTWZXQy9lTHpUYyszNmJCYnZnWkN6ZkVSK2pVSmZGdStCdXhaRmwycmla?=
 =?utf-8?B?VVJhTjhsSTR1NnBwU084TEwwWFFUTnNyZlZDeWVDSVRLME9HcUlsMDUxcXcz?=
 =?utf-8?B?NGVyVmJDNVpEV2NVbXBOTjNxN1lCWHFIS1AzQVozZTJ0eWRsSis3WHVJYXZt?=
 =?utf-8?B?c2kxRWsxZnJtV1FGeU1QcTRoZDMxRzM4ZjFXT1JoQS90bUtkbGwvWHIrSnFN?=
 =?utf-8?B?WWxIMWJ5Nk5BL3ZzWU9wUmhlckRJb1hBdXN1djJFaUo3UjRYSTdrVjFESE90?=
 =?utf-8?B?VzVDb0R3NnhHUUpzTGt2bXZmNzRBY0lDUTFJYk1La1I3TU5WbmlXVTVIV3JH?=
 =?utf-8?B?dUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 09e28125-fdaf-4532-e5b6-08dc43024c12
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Mar 2024 02:07:12.7483
 (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: keH0YhqVdGdQynMnd7FhZNg+WA+Ihk8VK6ESf27vaD51VNnaAdPfWLXgM3Pc61gwJ1U6pLKjGuYvFzZqhjMWZA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYYPR11MB8431
X-OriginatorOrg: intel.com


On 3/13/2024 2:46 AM, Matthew Wilcox wrote:
> On Tue, Mar 12, 2024 at 02:32:43PM -0400, Zi Yan wrote:
>> On 12 Mar 2024, at 12:38, Matthew Wilcox wrote:
>>> Folios with a positive refcount are
>>> removed from the per-node or per-cgroup list _at which point there is
>>> an undocumented assumption_ that they will not be removed from the
>>> local list because they have a positive refcount.
>>
>> But that sounds very subtle if not broken. As an outsider of
> 
> I merely deduced this requirement; I didn't come up with it ...
My understanding is that this requirement is because of just local
list in deferred_split_scan().

Using fbatch instead of local list here as your created for that
issue debugging can eliminate this subtlety?


Regards
Yin, Fengwei

> 
>> deferred_split_scan(), only !list_empty(folio->_deferred_list) is checked.
>> The condition can be true if the folio is on split_queue or
>> local list of deferred_split_scan() with elevated refcount. In that case,
>> the folio cannot be removed from the list (either split_queue or local list)
>> even if split_queue_lock is held, since local list manipulation is not under
>> split_queue_lock. This makes _deferred_list a one-way train to anyone
>> except deferred_split_scan(), namely folios can only be added into
>> _deferred_list until they are freed or split by deferred_split_scan().
>>
>> Is that intended? If yes, maybe we should document it. If not, using
>> split_queue_lock to protect local list, or more explicitly folio->_deferred_list
>> might be better?
> 
> To be fair, the folio can be split by anybody as
> split_huge_page_to_list_to_order() is careful to only manipulate the
> deferred list while the refcount is frozen at 0.  I'm still trying to
> figure out where to document this behaviour of the deferred list that
> someone (for example, your good self) would actually see it.
> 
> 
>