[PATCH] mm: Fix compile error when CONFIG_SHMEM is not set
Hugh Dickins
hughd at google.com
Wed Jun 4 07:03:18 UTC 2025
Adding Thomas Hellström, father of ttm_backup_backup_page():
Steve doesn't have CONFIG_SHMEM=y, so now gets a build error because
there's no shmem_writeout(); whereas before 6.16, backup_backup
writeback would have oopsed on calling NULL ram_aops.writepage
when CONFIG_SHMEM is not set.
On Tue, 3 Jun 2025, Steven Rostedt wrote:
> On Tue, 3 Jun 2025 10:54:49 -0700
> Linus Torvalds <torvalds at linux-foundation.org> wrote:
> > On Tue, 3 Jun 2025 at 10:26, Steven Rostedt <rostedt at goodmis.org> wrote:
> > >
> > > config DRM_TTM
> > > tristate
> > > - depends on DRM && MMU
> > > + depends on DRM && MMU && SHMEM
> >
> > Yeah, except I think you should just make it be
> >
> > depends on DRM && SHMEM
> >
> > because SHMEM already depends on MMU.
>
> Yeah, if I had made this a real patch I would have done that, but this was
> only for seeing it it would work.
Many in drivers/gpu/drm do already "select SHMEM",
so I came to think that
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -188,6 +188,7 @@ source "drivers/gpu/drm/display/Kconfig"
config DRM_TTM
tristate
depends on DRM && MMU
+ select SHMEM
help
GPU memory management subsystem for devices with multiple
GPU memory types. Will be enabled automatically if a device driver
would be the right answer.
But perhaps that adds bloat to kernels that don't need backup_backup
writeback, and some #ifdef CONFIG_SHMEMs in or around backup_backup
would be more to the point. Maybe add that "select SHMEM" line
before rc1, then refine ttm_backup.c with #ifdefs at leisure?
But I just don't appreciate backup_backup and its place in the
drm world: it's a matter for Thomas and dri-devel to work out.
>
> >
> > That said, our docs already say that if you disable SHMEM, it gets
> > replaced by RAMFS, so maybe just having a ramfs version is the
> > RightThing(tm).
> >
> > I don't think such a ramfs version should just return 0 - much less an
> > error. I think it should always redirty the page.
> >
> > IOW, I think the "ramfs" version should look something like
> >
> > folio_mark_dirty(folio);
> > if (wbc->for_reclaim)
> > return AOP_WRITEPAGE_ACTIVATE; /* Return with folio locked */
> > folio_unlock(folio);
> > return 0;
> >
> > which is what shmem does for the "page is locked" case.
>
> I'll let someone that understand the code a bit more than I do to make such
> a change. My patch was just a "this makes my system build" thing and let
> those that know this code do the RightThing(tm).
I did start out from the position that mm/shmem.c should provide a good
shmem_writeout() stub for the tiny !CONFIG_SHMEM case. But seeing as
nobody else has wanted it before, and backup_backup would have been
crashing in such a case, it now seems to me pointless to add such a stub.
Unless all those drm "select SHMEM"s got added long ago to avoid exactly
such crashes, and a shmem stub is preferred throughout drivers/gpu/drm.
I vote for the "select SHMEM", but Thomas and dri-devel and Linus
should decide.
Hugh
More information about the dri-devel
mailing list