Re: [PATCH 11/11] fs: Remove aops->writepage

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Mon, 2025-03-17 at 22:30 +0000, Matthew Wilcox wrote:
> On Mon, Mar 17, 2025 at 03:22:30AM +0000, Matthew Wilcox wrote:
> > On Sun, Mar 16, 2025 at 06:08:52PM -0700, Fan Ni wrote:
> > > On Fri, Mar 07, 2025 at 01:54:11PM +0000, Matthew Wilcox (Oracle)
> > > wrote:
> > > > All callers and implementations are now removed, so remove the
> > > > operation
> > > > and update the documentation to match.
> > > > 
> > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > > > ---
> > > 
> > > Hi Matthew,
> > > 
> > > Tried to apply the remaining patches in the patchest (Patch 5-
> > > 11)  which
> > > have not picked up by linux-next. It seems we have more to
> > > cleanup.
> > > 
> > > For example, I hit the following issue when try to compile
> > > ----------------------------------------------------------------
> > > drivers/gpu/drm/ttm/ttm_backup.c: In function
> > > ‘ttm_backup_backup_page’:
> > > drivers/gpu/drm/ttm/ttm_backup.c:139:39: error: ‘const struct
> > > address_space_operations’ has no member named ‘writepage’; did
> > > you mean ‘writepages’?
> > >   139 |                 ret = mapping->a_ops-
> > > >writepage(folio_file_page(to_folio, idx), &wbc);
> > 
> > Looks like that was added to linux-next after I completed the
> > removal of
> > ->writepage.  Thomas, what's going on here?
> 
> This patch fixes the compilation problem.  But I don't understand why
> it's messing with the reclaim flag.  Thomas, can you explain?

Hi, Sorry for not responding earlier. The patch that uses writepage()
here has been around for quite some time waiting for reviews / acks so
I failed to notice that it's going away.

Anyway the reclaim flag clearing follows that of pageout() in vmscan.c
which was also the case for the i915_gem_shmem.c usage in
__shmem_writeback(). My understanding was that if the writeback was
already completed at that point, the reclaim flag was no longer
desirable.

Let me know if this requires some action on my side. Unfortunately
freedesktop.org is down for maintainance, possibly for the whole week,
so there will be no drm subsystem PRs this week AFAICT.

The fix below looks good to me, BTW.

Thanks,
Thomas


> 
> +++ b/drivers/gpu/drm/ttm/ttm_backup.c
> @@ -136,13 +136,13 @@ ttm_backup_backup_page(struct ttm_backup
> *backup, struct page *page,
>                         .for_reclaim = 1,
>                 };
>                 folio_set_reclaim(to_folio);
> -               ret = mapping->a_ops-
> >writepage(folio_file_page(to_folio, idx), &wbc);
> +               ret = shmem_writeout(to_folio, &wbc);
>                 if (!folio_test_writeback(to_folio))
>                         folio_clear_reclaim(to_folio);
>                 /*
> -                * If writepage succeeds, it unlocks the folio.
> -                * writepage() errors are otherwise dropped, since
> writepage()
> -                * is only best effort here.
> +                * If writeout succeeds, it unlocks the folio. 
> errors
> +                * are otherwise dropped, since writeout is only best
> +                * effort here.
>                  */
>                 if (ret)
>                         folio_unlock(to_folio);
>