linux-kernel - Re: [PATCH v2 3/3] mm: memory-hotplug: check folio ref count first in do_migrate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3ebe55f4-99ff-4144-95cc-a8fcf682bc90@huawei.com>
Date: Tue, 21 Jan 2025 10:17:45 +0800
From: mawupeng <mawupeng1@...wei.com>
To: <linmiaohe@...wei.com>
CC: <mawupeng1@...wei.com>, <linux-mm@...ck.org>,
	<linux-kernel@...r.kernel.org>, <akpm@...ux-foundation.org>,
	<david@...hat.com>, <osalvador@...e.de>, <nao.horiguchi@...il.com>,
	<mhocko@...e.com>
Subject: Re: [PATCH v2 3/3] mm: memory-hotplug: check folio ref count first in
 do_migrate_rang



On 2025/1/20 14:32, Miaohe Lin wrote:
> On 2025/1/16 14:16, Wupeng Ma wrote:
>> From: Ma Wupeng <mawupeng1@...wei.com>
>>
>> If a folio has an increased reference count, folio_try_get() will acquire
>> it, perform necessary operations, and then release it. In the case of a
>> poisoned folio without an elevated reference count (which is unlikely for
>> memory-failure), folio_try_get() will simply bypass it.
>>
>> Therefore, relocate the folio_try_get() function, responsible for checking
>> and acquiring this reference count at first.
>>
>> Signed-off-by: Ma Wupeng <mawupeng1@...wei.com>
>> ---
>>  mm/memory_hotplug.c | 14 ++++----------
>>  1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 2815bd4ea483..3fb75ee185c6 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1786,6 +1786,9 @@ static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>>  		page = pfn_to_page(pfn);
>>  		folio = page_folio(page);
>>  
>> +		if (!folio_try_get(folio))
>> +			continue;
>> +
>>  		/*
>>  		 * No reference or lock is held on the folio, so it might
>>  		 * be modified concurrently (e.g. split).  As such,
>> @@ -1795,12 +1798,6 @@ static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>>  		if (folio_test_large(folio))
>>  			pfn = folio_pfn(folio) + folio_nr_pages(folio) - 1;
>>  
>> -		/*
>> -		 * HWPoison pages have elevated reference counts so the migration would
>> -		 * fail on them. It also doesn't make any sense to migrate them in the
>> -		 * first place. Still try to unmap such a page in case it is still mapped
>> -		 * (keep the unmap as the catch all safety net).
>> -		 */
>>  		if (folio_test_hwpoison(folio) ||
>>  		    (folio_test_large(folio) && folio_test_has_hwpoisoned(folio))) {
>>  			if (WARN_ON(folio_test_lru(folio)))
>> @@ -1811,12 +1808,9 @@ static void do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
>>  				folio_unlock(folio);
>>  			}
>>  
>> -			continue;
>> +			goto put_folio;
>>  		}
>>  
>> -		if (!folio_try_get(folio))
>> -			continue;
>> -
>>  		if (unlikely(page_folio(page) != folio))
>>  			goto put_folio;
> 
> Will it be necessary to move this check above folio_test_hwpoison trunk too?

Thanks.

AFAICT  we can do this, I'll move this in the next patch. there is no need to handle this page if
the state of this folio changes.

> 
> Thanks.
> .