Re: [PATCH] mm/huge_memory: fix NULL pointer deference when splitting shmem folio in swap cache

["David Hildenbrand (Red Hat)" <david@kernel.org>]

>> Given folio_test_swapcache() might have false positives,
>> I assume we'd need a
>>
>> 	folio_test_swapbacked() && folio_test_swapcache(folio)
>>
>> To detect large large shmem folios in the swapcache in all cases here.
>>
>> Something like the following would hopefully do:
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 2f2a521e5d683..57aab66bedbea 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3515,6 +3515,13 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>>          return ret;
>>   }
>>   +static bool folio_test_shmem_swapcache(struct folio *folio)
>> +{
>> +       VM_WARN_ON_ONCE_FOLIO(folio_test_anon(folio), folio);
>> +       /* These folios do not have folio->mapping set. */
>> +       return folio_test_swapbacked(folio) && folio_test_swapcache(folio);
>> +}
>> +
>>   bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>>                  bool warns)
>>   {
>> @@ -3524,6 +3531,9 @@ bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>>                                  "Cannot split to order-1 folio");
>>                  if (new_order == 1)
>>                          return false;
>> +       } else if (folio_test_shmem_swapcache(folio)) {
>> +               /* TODO: support shmem folios that are in the swapcache. */
>> +               return false;
> 
> With this, truncated shmem returns -EINVALID instead of -EBUSY now.
> Can s390_wiggle_split_folio() such folios?

[noting that s390_wiggle_split_folio() was just one caller where I new 
the return value differs. I suspect there might be more.]

I am still not clear on that one.

s390x obtains the folio while walking the page tables. In case it gets 
-EBUSY it simply retries to obtain the folio from the page tables.

So assuming there was concurrent truncation and we returned -EBUSY, it 
would just retry walking the page tables (trigger a fault to map a 
folio) and retry with that one.

I would assume that the shmem folio in the swapcache could never have 
worked before, and that there is no way to make progress really.

In other words: do we know how we can end up with a shmem folio that is 
in the swapcache and does not have folio->mapping set?

Could that think still be mapped into the page tables? (I hope not, but 
right now I am confused how that can happen )

-- 
Cheers

David