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 wrote: > > On Tue, 3 Jun 2025 at 10:26, Steven Rostedt 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