On Wed, Mar 06, 2024 at 10:39:37PM +0000, David Howells wrote:
> Here's a patch to have a go at getting rid of ->launder_folio(). Since it's
> failable and cannot guarantee that pages in the range are removed, I've tried
> to replace laundering with just flush-and-wait, dropping the folio lock around
> the I/O.
My sense is that ->launder_folio doesn't actually need to be replaced.
commit e3db7691e9f3dff3289f64e3d98583e28afe03db
Author: Trond Myklebust <[email protected]>
Date: Wed Jan 10 23:15:39 2007 -0800
[PATCH] NFS: Fix race in nfs_release_page()
NFS: Fix race in nfs_release_page()
invalidate_inode_pages2() may find the dirty bit has been set on a page
owing to the fact that the page may still be mapped after it was locked.
Only after the call to unmap_mapping_range() are we sure that the page
can no longer be dirtied.
In order to fix this, NFS has hooked the releasepage() method and tries
to write the page out between the call to unmap_mapping_range() and the
call to remove_mapping(). This, however leads to deadlocks in the page
reclaim code, where the page may be locked without holding a reference
to the inode or dentry.
Fix is to add a new address_space_operation, launder_page(), which will
attempt to write out a dirty page without releasing the page lock.
Signed-off-by: Trond Myklebust <[email protected]>
I don't understand why this couldn't've been solved by page_mkwrite.
NFS did later add nfs_vm_page_mkwrite in July 2007, and maybe it's just
not needed any more? I haven't looked into it enough to make sure,
but my belief is that we should be able to get rid of it.
Matthew Wilcox <[email protected]> wrote:
> commit e3db7691e9f3dff3289f64e3d98583e28afe03db
> Author: Trond Myklebust <[email protected]>
> Date: Wed Jan 10 23:15:39 2007 -0800
>
> [PATCH] NFS: Fix race in nfs_release_page()
>...
> invalidate_inode_pages2() may find the dirty bit has been set on a page
> owing to the fact that the page may still be mapped after it was locked.
> Only after the call to unmap_mapping_range() are we sure that the page
> can no longer be dirtied.
Is that last sentence even true? It evicts folios from the TLB and/or
pagetables, but it doesn't actually trim any mmap made - in which case,
userspace is perfectly at liberty to regenerate and dirty the folio the moment
the folio is removed from the page tables. Otherwise DIO-to/from-mmap will
deadlock.
> but my belief is that we should be able to get rid of it.
I think you're probably correct. The best we can do, I think, is to preface
any call to invalidate_inode_pages2() with a flush-and-wait over the same
range.
David