Received-SPF: pass (google.com: domain of linux-kernel+bounces-101053-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <feb2d3c1-d890-473b-8a8d-bdba11ba70c9@intel.com>
Date: Wed, 13 Mar 2024 10:33:54 +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
From: "Yin, Fengwei" <fengwei.yin@intel.com>
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>
 <b5017375-d43b-44f9-b931-3046ebead9cf@intel.com>
In-Reply-To: <b5017375-d43b-44f9-b931-3046ebead9cf@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Z29SSlU4YmFrR2JTVzA2c25yc1FRN0VoK2gxaUlJN2pla1J1dWpxbTZUcUJm?=
 =?utf-8?B?bFZiWjBobFNKMTFaczhyTHlRTjJFU0lacHRnRVFuV3oyS1h4eG03b0lockdq?=
 =?utf-8?B?alp0eUFlUUdiZHJ5QnBZeW5ab1owcjZsUVpqc1hCc0FVZlZHTnF4Vy9PMTY2?=
 =?utf-8?B?c0ZDajZWMDkybnBKeHpjQlBBTnhQbVNUS1JFTklPRFhMc1RUZUJSa0JoOW5U?=
 =?utf-8?B?SHJ0aFRiMFpuLzhiRGxRR01GSEVTUDgvM2lxZk1IWlZIQTh5d0tyYTROM0Zv?=
 =?utf-8?B?UVN4bkJMRVdQdnhrR2NXdi9oU2tGZkxtREtuVDNXM0JzcEhjR0JDY0JEQ0FG?=
 =?utf-8?B?dmh0eUJreklvVUR6WnlhRWIyS0NkVXhIRHJIQlUxUlYzMXlUSldQUWZ0Z2VU?=
 =?utf-8?B?WW94YmxISC8vNDZnUHBac0crRnZxRkoyZzRwR1h6b2pML0M2bXRwWmtUSDBt?=
 =?utf-8?B?S3hFZ0hjU01sMnhkN0tNYjNJYWc2RnVTeDRnZFpqU0tOYlIycENZL0pZNTdz?=
 =?utf-8?B?VFlhcnpFZ01tQlgwZ1laUGZHWkRjVS9VWTFVenc4UW84ZW8rbjdsVThORkIy?=
 =?utf-8?B?NVMrUERWU0xha3IrY3hVckZCeWQ4WXdQMnJoME5iQ09ibk1SODNCMmkxZWoy?=
 =?utf-8?B?UEhPQ3E2WEhYdXpFNWYyemQzdzNBSzZtVlNJb3M0OU9WTG51QTNDOXJNVFlT?=
 =?utf-8?B?RmdUYWFwMVYxbjVyRTRTUTJGMGpIbTJocWJGL1VTWmJmSGI0SzRPdm9DK1hQ?=
 =?utf-8?B?Z2lsaDJ1MVlvMk9qeUIrb21oTE1QNzF0VEM4ZUhrbWZWbXFFL3lZb05pd2J6?=
 =?utf-8?B?NWNveVU2azlsOUxFL2JJZjg2UUs3b3dNei8yUmQrc3RqN2hRaWJCWlBHYVV3?=
 =?utf-8?B?eG9PcVorUWJCS29xenMyVmFuaFRHZGNlLzZ0RDUzY1h4NDlsQ0hneVFYdjhW?=
 =?utf-8?B?OWtib0RxbXBMdk5kSTE1MWpMMzI4Z3J2aXE5K0dFR25sU2xVTHU4KzJDQ3pG?=
 =?utf-8?B?bFI3enRRTlVzZVU3RGlJcGJYYkQwYVgvVGJ2aGJ1a0NIeXlkYVNRUCtKVStQ?=
 =?utf-8?B?WDRPQ0d0YXk1VnN5WFJ2UjVEdEF3UVA4WFk1ZU9kYmNEOC8ybWdXeFUvWGc3?=
 =?utf-8?B?K3Vvd0ZERjhYMGY5MW16VnhLN0RidFZuR25rSm9jTnVNRUh5UGxpREg2aEZL?=
 =?utf-8?B?dWRDNDJyQlVpUHpoeXhVYjNtNWF0M21vV3BqVlNRbTE5Z3hFQkZTYXpyY1Jj?=
 =?utf-8?B?QkVzMEhHNTdFKzhNTGlFOVhQWjRnc0lnK0FOb1dnUmV1dEF2OThnK1l3TUVi?=
 =?utf-8?B?dVl4ak9rTmkvVFRNOGlxTmdFVSsyWkdUM1lmMUYwU3QvT0xiUHpGdndNbFQr?=
 =?utf-8?B?N0JvQXZtRGJQMWhCQ29DaVcvcFgwN3ZtcGhVaEcwb0dFRkRad2ZQSjhMbUxG?=
 =?utf-8?B?dTBkdjU5QlB5ejN5MHU4c2ZEZmVKYmFJR3hjY1R0SmNuYXhpcDN5T3YvOEdB?=
 =?utf-8?B?bTk2NS9uNlI0Y3BucFV0Yjc0RjQ2S2liRzVKTVZsQVdieFBzQTh5SjNkeGZT?=
 =?utf-8?B?V3prY3NWL3B3VERJZTdiU1d0aUVBeWxucjZOTXhXRU5YcndnL1gyYXpyVzFr?=
 =?utf-8?B?bXJDQ1JhK2l2MDZYa2p4N1VFRmdGazdreHo0Y0FnT2dUTENCbGlXKzJ0YVJX?=
 =?utf-8?B?VnlzcHZWNXJpMjZPUzVNNVp4cGZPSnEwenEzNlJjcHdkVitDWnRDTllkdFRF?=
 =?utf-8?B?SHUwRmpxNnhCU2lKRkdnUmJPYnI4c0U0NmtsL3haSlRuc0ptd1JoWlZJYWtG?=
 =?utf-8?B?TEs2WEdiUXlzU1hrRnlRSnYxUGxRZGJ6dThUcVNzczFDeUFDK2d0QVo2TVV5?=
 =?utf-8?B?TnVPN21LUng2MVhncHQyVU52SFdHckFVR3kzemxZS3JsV3l6RzNaZDV0M2tT?=
 =?utf-8?B?Q1daM2g0OHdvODd3TTd3cHY1L0grc2xaYzdtYkNDWElTWW5QcHEvSXliY1FS?=
 =?utf-8?B?ZHpQdGZRRFRLVVdUWDZqTC9rZ1hpZ040QW9aVG1wM1l3Q3k5Rm91RHpqdG85?=
 =?utf-8?B?OW1GcmR4KzA1RjNiSnlTRkpJV0pKdUVsbmtSaTdEVkNGUmRPcU16QmNWOEdY?=
 =?utf-8?B?c0l0K2xwUE9nNEFzcnF5QzFseW05MEI0bXIzWUFqUlQ5LzV6Q1VyenBNNW45?=
 =?utf-8?B?eHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 23d47aff-104d-4225-4c0b-08dc43060ae6
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:34:01.4192
 (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: vs+r9gJMBxLvMZwkKiwQjCLdKm72cLQi1760mPDYY30DO1931Y6RTDGdOL6mXqBtcabGlEQNLNaZY9+lbkpxQg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5065
X-OriginatorOrg: intel.com


On 3/13/2024 10:07 AM, Yin, Fengwei wrote:
> 
> 
> 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?
May not good idea because it's possible the folios in fbatch can
be removed from deferred_list by migration.

> 
> 
> 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.
>>
>>
>>
>