Re: [PATCH] mm: Fix compile error when CONFIG_SHMEM is not set

[Hugh Dickins]

On Mon, 2 Jun 2025, Matthew Wilcox wrote:
> On Mon, Jun 02, 2025 at 05:14:58PM -0400, Steven Rostedt wrote:
> > On Mon, 2 Jun 2025 17:05:00 -0400
> > Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> > 
> > > From: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > > 
> > > When CONFIG_SHMEM is not set, the following compiler error occurs:
> > > 
> > > ld: vmlinux.o: in function `ttm_backup_backup_page':
> > 
> > I'm not sure this is the right fix or not.
> 
> > > +int shmem_writeout(struct folio *folio, struct writeback_control *wbc)
> > > +{
> > > +	return 0;
> > 
> > Perhaps this should return:
> > 
> > 	return swap_writeout(folio, wbc);
> 
> I don't think so.  ttm_backup_backup_page() gets its page from:
> 
>         to_folio = shmem_read_folio_gfp(mapping, idx, alloc_gfp);
> ...
>                 ret = shmem_writeout(to_folio, &wbc);
> 
> and if you look at the implementation of shmem_read_folio_gfp() does:
> 
> #else
>         /*
>          * The tiny !SHMEM case uses ramfs without swap
>          */
>         return mapping_read_folio_gfp(mapping, index, gfp);
> #endif
> 
> so I would say that if anybody is actually using it this way (and 99%
> chance they're not), they literally cannot write back the folio.  So
> I think your initial patch is fine.

Agreed that ramfs does not use swap, so calling swap_writepage() would
be weird.  But, thanks for the build fix Steve, but it cannot be right
because return 0 says shmem_writeout() successfully sent the page to
swap, and that has unlocked the page (or soon will do so).  It should
return an error (-ENXIO?), but I haven't checked what the callers do with
that, nor whether they need the folio to be redirtied.  And wouldn't it
need an EXPORT like the real one?  Sorry, can't keep up, there are many
many things I should have looked at but have not... Tomorrow?

Hugh