Re: [PATCH v2 1/2] mm: always call rmap_walk() on locked folios

[David Hildenbrand <david@redhat.com>]

>>    
>>    static ssize_t page_idle_bitmap_read(struct file *file, struct kobject *kobj,
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 0bc7cf8b7359..fd9f18670440 100644
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -489,17 +489,15 @@ void __init anon_vma_init(void)
>>     * if there is a mapcount, we can dereference the anon_vma after observing
>>     * those.
>>     *
>> - * NOTE: the caller should normally hold folio lock when calling this.  If
>> - * not, the caller needs to double check the anon_vma didn't change after
>> - * taking the anon_vma lock for either read or write (UFFDIO_MOVE can modify it
>> - * concurrently without folio lock protection). See folio_lock_anon_vma_read()
>> - * which has already covered that, and comment above remap_pages().
>> + * NOTE: the caller should hold folio lock when calling this.
>>     */
>>    struct anon_vma *folio_get_anon_vma(const struct folio *folio)
>>    {
>>    	struct anon_vma *anon_vma = NULL;
>>    	unsigned long anon_mapping;
>>    
>> +	VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
>> +
> 
> As raised in v1, I am not sure about device-private dax folios that are
> anonymous where we could end up here through
> memory_failure_dev_pagemap() and not having the folio locked.

As discussed in the v1 thread, we should be bailing out for 
device_private folios earlier, and IIUC, these are the only ones that 
can be anon. Likely there is still something shaky with anon folios 
earlier on that path, but that's not related to this series.

So let's see if this patch will reveal any unexpected surprises

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb