On Mon, 2025-08-18 at 16:21 +0100, Matthew Wilcox wrote:
> On Mon, Aug 18, 2025 at 07:39:50AM -0700, Trond Myklebust wrote:
> > @@ -349,8 +349,12 @@ static void nfs_folio_end_writeback(struct
> > folio *folio)
> > static void nfs_page_end_writeback(struct nfs_page *req)
> > {
> > if (nfs_page_group_sync_on_bit(req, PG_WB_END)) {
> > + struct folio *folio = nfs_page_to_folio(req);
> > +
> > + if (folio_test_clear_dropbehind(folio))
> > + set_bit(PG_DROPBEHIND, &req->wb_head-
> > >wb_flags);
> > nfs_unlock_request(req);
> > @@ -787,8 +791,15 @@ static void nfs_inode_remove_request(struct
> > nfs_page *req)
> > clear_bit(PG_MAPPED, &req->wb_head-
> > >wb_flags);
> > }
> > spin_unlock(&mapping->i_private_lock);
> > - }
> > - nfs_page_group_unlock(req);
> > + nfs_page_group_unlock(req);
> > +
> > + if (test_and_clear_bit(PG_DROPBEHIND,
> > + &req->wb_head->wb_flags)) {
> > + folio_set_dropbehind(folio);
> > + folio_end_dropbehind(folio);
> > + }
>
> I don't think this technique is "safe". By clearing the flag early,
> the page cache can't see that a folio that was created by dropbehind
> has now been reused and should have its dropbehind flag cleared. So
> we
> might see pages dropped from the cache that really should not be.
The only alternative would be to add back in a helper in mm/filemap.c
that does the normal folio_end_writeback() routine, but ignores the
dropbehind flag. (folio_end_writeback_ignore_dropbehind()?)
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@xxxxxxxxxx, trond.myklebust@xxxxxxxxxxxxxxx
