linux-kernel - Re: [PATCH 2/2] mm/truncate: don't skip dirty page in folio_unmap

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: <fefdb1f7-bf14-4bae-8666-132d1ceaed66@linux.alibaba.com>
Date: Wed, 19 Feb 2025 09:55:08 +0800
From: Jingbo Xu <jefflexu@...ux.alibaba.com>
To: Dave Chinner <david@...morbit.com>
Cc: axboe@...nel.dk, linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
 brauner@...nel.org, linux-kernel@...r.kernel.org, viro@...iv.linux.org.uk
Subject: Re: [PATCH 2/2] mm/truncate: don't skip dirty page in
 folio_unmap_invalidate()



On 2/19/25 8:11 AM, Dave Chinner wrote:
> On Tue, Feb 18, 2025 at 08:02:09PM +0800, Jingbo Xu wrote:
>> ... otherwise this is a behavior change for the previous callers of
>> invalidate_complete_folio2(), e.g. the page invalidation routine.
>>
>> Fixes: 4a9e23159fd3 ("mm/truncate: add folio_unmap_invalidate() helper")
>> Signed-off-by: Jingbo Xu <jefflexu@...ux.alibaba.com>
>> ---
>>  mm/truncate.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/mm/truncate.c b/mm/truncate.c
>> index e2e115adfbc5..76d8fcd89bd0 100644
>> --- a/mm/truncate.c
>> +++ b/mm/truncate.c
>> @@ -548,8 +548,6 @@ int folio_unmap_invalidate(struct address_space *mapping, struct folio *folio,
>>  
>>  	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
>>  
>> -	if (folio_test_dirty(folio))
>> -		return 0;
> 
> Shouldn't that actually return -EBUSY because the folio could not be
> invalidated?
> 
> Indeed, further down the function the folio gets locked and the
> dirty test is repeated. If it fails there it returns -EBUSY....
> 
> -Dave.

Yeah, the original logic of invalidate_inode_pages2_range() is like

```
invalidate_inode_pages2_range
  # lock page
  # launder the page if it's dirty

  invalidate_complete_folio2
    # recheck if it's dirty, and skip the dirty page (no idea how page
could be redirtied after launder_page())
```

while after commit 4a9e23159fd3 ("mm/truncate: add
folio_unmap_invalidate() helper"), this logic is changed to:

```
invalidate_inode_pages2_range
  # lock page
  folio_unmap_invalidate
    # check if it's dirty, and skip dirty page
    # launder the page if it's dirty (doubt if it's noops)

    # recheck if it's dirty, and skip the dirty page
```


-- 
Thanks,
Jingbo