Re: [PATCH v2] mm/memory-failure: fix concurrent access issue in min_order_for_split()

"David Hildenbrand (Arm)" <[email protected]>
Newsgroups org.kernel.vger.stable,org.kvack.linux-mm
Message-ID <[email protected]>
>>  	/*
>> @@ -2862,8 +2841,7 @@ static int soft_offline_in_use_page(struct page *page)
>>  	};
>>
>>  	if (!huge && folio_test_large(folio)) {
>> -		const int new_order = min_order_for_split(folio);
>> -
>> +		lock_page(page);
>>  		/*
>>  		 * If new_order (target split order) is not 0, do not split the
>>  		 * folio at all to retain the still accessible large folio.
>> @@ -2871,8 +2849,14 @@ static int soft_offline_in_use_page(struct page *page)
>>  		 * preferred, split it to non-zero new_order like it is done in
>>  		 * memory_failure().
>>  		 */
>> -		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
>> -						       /* release= */ true)) {
>> +		if (!min_order_for_split(folio))
>> +			ret = split_huge_page_to_order(page, 0);
>> +		else
>> +			ret = -EBUSY;
>> +		unlock_page(page);
>> +
>> +		if (ret) {
>> +			put_page(page);
>>  			pr_info("%#lx: thp split failed\n", pfn);
>>  			return -EBUSY;
> 
> Maybe return ret here? Seems a bit pointless to set ret, gate on it then not
> return it (even if result is the same)

I didn't check whether a caller actually relies on -EBUSY. So I decided to leave
the behavior unchanged.

-- 
Cheers,

David
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.