* [PATCH 0/2] drm/ttm: Improve the TTM operation context gfp_retry_mayfail behaviour
@ 2026-02-27 16:00 Thomas Hellström
2026-02-27 16:00 ` [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure Thomas Hellström
2026-02-27 16:00 ` [PATCH 2/2] drm/ttm: Avoid invoking the OOM killer when reading back swapped content Thomas Hellström
0 siblings, 2 replies; 8+ messages in thread
From: Thomas Hellström @ 2026-02-27 16:00 UTC (permalink / raw)
To: intel-xe
Cc: Thomas Hellström, Matthew Brost, Matthew Auld,
Christian König, dri-devel
Two small patches around the gfp_retry_mayfail behaviour:
Patch 1 ensures we don't spam the kernel log on allocation failures.
Patch 2 extends the gfp_retry_mayfail to swap readback.
Thomas Hellström (2):
drm/ttm: Don't spam the log on buffer object backing store allocation
failure
drm/ttm: Avoid invoking the OOM killer when reading back swapped
content
drivers/gpu/drm/ttm/ttm_backup.c | 6 ++++--
drivers/gpu/drm/ttm/ttm_pool.c | 7 +++++--
include/drm/ttm/ttm_backup.h | 2 +-
3 files changed, 10 insertions(+), 5 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure 2026-02-27 16:00 [PATCH 0/2] drm/ttm: Improve the TTM operation context gfp_retry_mayfail behaviour Thomas Hellström @ 2026-02-27 16:00 ` Thomas Hellström 2026-02-27 21:01 ` Matthew Brost 2026-03-02 9:02 ` Christian König 2026-02-27 16:00 ` [PATCH 2/2] drm/ttm: Avoid invoking the OOM killer when reading back swapped content Thomas Hellström 1 sibling, 2 replies; 8+ messages in thread From: Thomas Hellström @ 2026-02-27 16:00 UTC (permalink / raw) To: intel-xe Cc: Thomas Hellström, Matthew Brost, Matthew Auld, Christian König, dri-devel If the struct ttm_operation_ctx::gfp_retry_mayfail is true, buffer object backing store allocation failures are expected to silently fail with an error code to the caller. But currently an elaborate warning is printed to the system log. Don't spam the log in this way. Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> --- drivers/gpu/drm/ttm/ttm_pool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c index c0d95559197c..8fa9e09f6ee5 100644 --- a/drivers/gpu/drm/ttm/ttm_pool.c +++ b/drivers/gpu/drm/ttm/ttm_pool.c @@ -726,7 +726,7 @@ static int __ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt, gfp_flags |= __GFP_ZERO; if (ctx->gfp_retry_mayfail) - gfp_flags |= __GFP_RETRY_MAYFAIL; + gfp_flags |= __GFP_RETRY_MAYFAIL | __GFP_NOWARN; if (ttm_pool_uses_dma32(pool)) gfp_flags |= GFP_DMA32; -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure 2026-02-27 16:00 ` [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure Thomas Hellström @ 2026-02-27 21:01 ` Matthew Brost 2026-03-02 9:02 ` Christian König 1 sibling, 0 replies; 8+ messages in thread From: Matthew Brost @ 2026-02-27 21:01 UTC (permalink / raw) To: Thomas Hellström Cc: intel-xe, Matthew Auld, Christian König, dri-devel On Fri, Feb 27, 2026 at 05:00:11PM +0100, Thomas Hellström wrote: > If the struct ttm_operation_ctx::gfp_retry_mayfail is true, > buffer object backing store allocation failures are expected to > silently fail with an error code to the caller. But currently an > elaborate warning is printed to the system log. > > Don't spam the log in this way. > > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Reviewed-by: Matthew Brost <matthew.brost@intel.com> > --- > drivers/gpu/drm/ttm/ttm_pool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > index c0d95559197c..8fa9e09f6ee5 100644 > --- a/drivers/gpu/drm/ttm/ttm_pool.c > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > @@ -726,7 +726,7 @@ static int __ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt, > gfp_flags |= __GFP_ZERO; > > if (ctx->gfp_retry_mayfail) > - gfp_flags |= __GFP_RETRY_MAYFAIL; > + gfp_flags |= __GFP_RETRY_MAYFAIL | __GFP_NOWARN; > > if (ttm_pool_uses_dma32(pool)) > gfp_flags |= GFP_DMA32; > -- > 2.53.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure 2026-02-27 16:00 ` [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure Thomas Hellström 2026-02-27 21:01 ` Matthew Brost @ 2026-03-02 9:02 ` Christian König 2026-03-02 9:39 ` Thomas Hellström 1 sibling, 1 reply; 8+ messages in thread From: Christian König @ 2026-03-02 9:02 UTC (permalink / raw) To: Thomas Hellström, intel-xe; +Cc: Matthew Brost, Matthew Auld, dri-devel On 2/27/26 17:00, Thomas Hellström wrote: > If the struct ttm_operation_ctx::gfp_retry_mayfail is true, > buffer object backing store allocation failures are expected to > silently fail with an error code to the caller. But currently an > elaborate warning is printed to the system log. > > Don't spam the log in this way. That was intentionally removed or never added because Simona absolutely didn't liked the gfp_retry_mayfail flag at that time. In general I'm fine with this change since I think we have proved by now that the flag is useful, but that probably need more wider discussion. Regards, Christian. > > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > --- > drivers/gpu/drm/ttm/ttm_pool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > index c0d95559197c..8fa9e09f6ee5 100644 > --- a/drivers/gpu/drm/ttm/ttm_pool.c > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > @@ -726,7 +726,7 @@ static int __ttm_pool_alloc(struct ttm_pool *pool, struct ttm_tt *tt, > gfp_flags |= __GFP_ZERO; > > if (ctx->gfp_retry_mayfail) > - gfp_flags |= __GFP_RETRY_MAYFAIL; > + gfp_flags |= __GFP_RETRY_MAYFAIL | __GFP_NOWARN; > > if (ttm_pool_uses_dma32(pool)) > gfp_flags |= GFP_DMA32; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure 2026-03-02 9:02 ` Christian König @ 2026-03-02 9:39 ` Thomas Hellström 2026-03-09 9:36 ` Simona Vetter 0 siblings, 1 reply; 8+ messages in thread From: Thomas Hellström @ 2026-03-02 9:39 UTC (permalink / raw) To: Christian König, intel-xe; +Cc: Matthew Brost, Matthew Auld, dri-devel On Mon, 2026-03-02 at 10:02 +0100, Christian König wrote: > On 2/27/26 17:00, Thomas Hellström wrote: > > If the struct ttm_operation_ctx::gfp_retry_mayfail is true, > > buffer object backing store allocation failures are expected to > > silently fail with an error code to the caller. But currently an > > elaborate warning is printed to the system log. > > > > Don't spam the log in this way. > > That was intentionally removed or never added because Simona > absolutely didn't liked the gfp_retry_mayfail flag at that time. > > In general I'm fine with this change since I think we have proved by > now that the flag is useful, but that probably need more wider > discussion. Well for system memory it is a bit questionable to be honest, I think mostly because even if we return an error, the OOM killer might be invoked on an unrelated allocation immediately afterwards. Still, even if the use of gfp_retry_mayfail can be discussed, I'm not sure why an error here needs to be printed when there are a number of other errors that are not printed or printed only on debug. Thanks, Thomas > > Regards, > Christian. > > > > > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > > --- > > drivers/gpu/drm/ttm/ttm_pool.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c > > b/drivers/gpu/drm/ttm/ttm_pool.c > > index c0d95559197c..8fa9e09f6ee5 100644 > > --- a/drivers/gpu/drm/ttm/ttm_pool.c > > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > > @@ -726,7 +726,7 @@ static int __ttm_pool_alloc(struct ttm_pool > > *pool, struct ttm_tt *tt, > > gfp_flags |= __GFP_ZERO; > > > > if (ctx->gfp_retry_mayfail) > > - gfp_flags |= __GFP_RETRY_MAYFAIL; > > + gfp_flags |= __GFP_RETRY_MAYFAIL | __GFP_NOWARN; > > > > if (ttm_pool_uses_dma32(pool)) > > gfp_flags |= GFP_DMA32; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure 2026-03-02 9:39 ` Thomas Hellström @ 2026-03-09 9:36 ` Simona Vetter 0 siblings, 0 replies; 8+ messages in thread From: Simona Vetter @ 2026-03-09 9:36 UTC (permalink / raw) To: Thomas Hellström Cc: Christian König, intel-xe, Matthew Brost, Matthew Auld, dri-devel On Mon, Mar 02, 2026 at 10:39:10AM +0100, Thomas Hellström wrote: > On Mon, 2026-03-02 at 10:02 +0100, Christian König wrote: > > On 2/27/26 17:00, Thomas Hellström wrote: > > > If the struct ttm_operation_ctx::gfp_retry_mayfail is true, > > > buffer object backing store allocation failures are expected to > > > silently fail with an error code to the caller. But currently an > > > elaborate warning is printed to the system log. > > > > > > Don't spam the log in this way. > > > > That was intentionally removed or never added because Simona > > absolutely didn't liked the gfp_retry_mayfail flag at that time. > > > > In general I'm fine with this change since I think we have proved by > > now that the flag is useful, but that probably need more wider > > discussion. > > Well for system memory it is a bit questionable to be honest, I think > mostly because even if we return an error, the OOM killer might be > invoked on an unrelated allocation immediately afterwards. > > Still, even if the use of gfp_retry_mayfail can be discussed, I'm not > sure why an error here needs to be printed when there are a number of > other errors that are not printed or printed only on debug. Yeah adding the NOWARN makes sense irrespective of the bigger question, so on that: Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch> For the mayfail I have honestly no recollection anymore of that, but making a guess I wasn't a fan because way back it was used to hack around locking inversions between alloc and reclaim paths. Much more with dev->struct_mutex drivers before moving to dma_resv for non-ttm drivers too. And that's not great. Plus GL userspace did not cope with alloc failures, so punting to the OOM killer like for everything else made sense. And hence there was really no use for this. But with vk and other low-level gpu apis that changed, we do want to just pass ENOMEM to userspace now in many conditions. I think best would be to add a patch to this series to document when gfp_retry_mayfail can be used (userspace expects the kernel to pass alloc failures up the stack) and must not be used (hacking around locking inversions with reclaim) and then ship this. Might also be a good excuse to switch the kerneldoc for struct ttm_operation_ctx over to the inline style so we can be appropriately verbose. But yeah, going through current users (on a Monday morning without coffee) I think the flag has solid users by now and there's no fundamental objections from me. Cheers, Sima > > Thanks, > Thomas > > > > > > Regards, > > Christian. > > > > > > > > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > > > --- > > > drivers/gpu/drm/ttm/ttm_pool.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c > > > b/drivers/gpu/drm/ttm/ttm_pool.c > > > index c0d95559197c..8fa9e09f6ee5 100644 > > > --- a/drivers/gpu/drm/ttm/ttm_pool.c > > > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > > > @@ -726,7 +726,7 @@ static int __ttm_pool_alloc(struct ttm_pool > > > *pool, struct ttm_tt *tt, > > > gfp_flags |= __GFP_ZERO; > > > > > > if (ctx->gfp_retry_mayfail) > > > - gfp_flags |= __GFP_RETRY_MAYFAIL; > > > + gfp_flags |= __GFP_RETRY_MAYFAIL | __GFP_NOWARN; > > > > > > if (ttm_pool_uses_dma32(pool)) > > > gfp_flags |= GFP_DMA32; -- Simona Vetter Software Engineer http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/ttm: Avoid invoking the OOM killer when reading back swapped content 2026-02-27 16:00 [PATCH 0/2] drm/ttm: Improve the TTM operation context gfp_retry_mayfail behaviour Thomas Hellström 2026-02-27 16:00 ` [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure Thomas Hellström @ 2026-02-27 16:00 ` Thomas Hellström 2026-03-10 14:10 ` Maarten Lankhorst 1 sibling, 1 reply; 8+ messages in thread From: Thomas Hellström @ 2026-02-27 16:00 UTC (permalink / raw) To: intel-xe Cc: Thomas Hellström, Matthew Brost, Matthew Auld, Christian König, dri-devel In situations where the system is very short on RAM, the shmem readback from swap-space may invoke the OOM killer. However, since this might be a recoverable situation where the caller is indicating this by setting struct ttm_operation_ctx::gfp_retry_mayfail to true, adjust the gfp value used by the allocation accordingly. Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> --- drivers/gpu/drm/ttm/ttm_backup.c | 6 ++++-- drivers/gpu/drm/ttm/ttm_pool.c | 5 ++++- include/drm/ttm/ttm_backup.h | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_backup.c b/drivers/gpu/drm/ttm/ttm_backup.c index 6bd4c123d94c..81df4cb5606b 100644 --- a/drivers/gpu/drm/ttm/ttm_backup.c +++ b/drivers/gpu/drm/ttm/ttm_backup.c @@ -44,18 +44,20 @@ void ttm_backup_drop(struct file *backup, pgoff_t handle) * @dst: The struct page to copy into. * @handle: The handle returned when the page was backed up. * @intr: Try to perform waits interruptible or at least killable. + * @additional_gfp: GFP mask to add to the default GFP mask if any. * * Return: 0 on success, Negative error code on failure, notably * -EINTR if @intr was set to true and a signal is pending. */ int ttm_backup_copy_page(struct file *backup, struct page *dst, - pgoff_t handle, bool intr) + pgoff_t handle, bool intr, gfp_t additional_gfp) { struct address_space *mapping = backup->f_mapping; struct folio *from_folio; pgoff_t idx = ttm_backup_handle_to_shmem_idx(handle); - from_folio = shmem_read_folio(mapping, idx); + from_folio = shmem_read_folio_gfp(mapping, idx, mapping_gfp_mask(mapping) + | additional_gfp); if (IS_ERR(from_folio)) return PTR_ERR(from_folio); diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c index 8fa9e09f6ee5..aa41099c5ecf 100644 --- a/drivers/gpu/drm/ttm/ttm_pool.c +++ b/drivers/gpu/drm/ttm/ttm_pool.c @@ -530,6 +530,8 @@ static int ttm_pool_restore_commit(struct ttm_pool_tt_restore *restore, p = first_page[i]; if (ttm_backup_page_ptr_is_handle(p)) { unsigned long handle = ttm_backup_page_ptr_to_handle(p); + gfp_t additional_gfp = ctx->gfp_retry_mayfail ? + __GFP_RETRY_MAYFAIL | __GFP_NOWARN : 0; if (IS_ENABLED(CONFIG_FAULT_INJECTION) && ctx->interruptible && should_fail(&backup_fault_inject, 1)) { @@ -543,7 +545,8 @@ static int ttm_pool_restore_commit(struct ttm_pool_tt_restore *restore, } ret = ttm_backup_copy_page(backup, restore->alloced_page + i, - handle, ctx->interruptible); + handle, ctx->interruptible, + additional_gfp); if (ret) break; diff --git a/include/drm/ttm/ttm_backup.h b/include/drm/ttm/ttm_backup.h index c33cba111171..29b9c855af77 100644 --- a/include/drm/ttm/ttm_backup.h +++ b/include/drm/ttm/ttm_backup.h @@ -56,7 +56,7 @@ ttm_backup_page_ptr_to_handle(const struct page *page) void ttm_backup_drop(struct file *backup, pgoff_t handle); int ttm_backup_copy_page(struct file *backup, struct page *dst, - pgoff_t handle, bool intr); + pgoff_t handle, bool intr, gfp_t additional_gfp); s64 ttm_backup_backup_page(struct file *backup, struct page *page, -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/ttm: Avoid invoking the OOM killer when reading back swapped content 2026-02-27 16:00 ` [PATCH 2/2] drm/ttm: Avoid invoking the OOM killer when reading back swapped content Thomas Hellström @ 2026-03-10 14:10 ` Maarten Lankhorst 0 siblings, 0 replies; 8+ messages in thread From: Maarten Lankhorst @ 2026-03-10 14:10 UTC (permalink / raw) To: Thomas Hellström, intel-xe Cc: Matthew Brost, Matthew Auld, Christian König, dri-devel Hey, Den 2026-02-27 kl. 17:00, skrev Thomas Hellström: > In situations where the system is very short on RAM, the shmem > readback from swap-space may invoke the OOM killer. > > However, since this might be a recoverable situation where the caller > is indicating this by setting > struct ttm_operation_ctx::gfp_retry_mayfail to true, adjust the gfp > value used by the allocation accordingly. > > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > --- > drivers/gpu/drm/ttm/ttm_backup.c | 6 ++++-- > drivers/gpu/drm/ttm/ttm_pool.c | 5 ++++- > include/drm/ttm/ttm_backup.h | 2 +- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_backup.c b/drivers/gpu/drm/ttm/ttm_backup.c > index 6bd4c123d94c..81df4cb5606b 100644 > --- a/drivers/gpu/drm/ttm/ttm_backup.c > +++ b/drivers/gpu/drm/ttm/ttm_backup.c > @@ -44,18 +44,20 @@ void ttm_backup_drop(struct file *backup, pgoff_t handle) > * @dst: The struct page to copy into. > * @handle: The handle returned when the page was backed up. > * @intr: Try to perform waits interruptible or at least killable. > + * @additional_gfp: GFP mask to add to the default GFP mask if any. > * > * Return: 0 on success, Negative error code on failure, notably > * -EINTR if @intr was set to true and a signal is pending. > */ > int ttm_backup_copy_page(struct file *backup, struct page *dst, > - pgoff_t handle, bool intr) > + pgoff_t handle, bool intr, gfp_t additional_gfp) > { > struct address_space *mapping = backup->f_mapping; > struct folio *from_folio; > pgoff_t idx = ttm_backup_handle_to_shmem_idx(handle); > > - from_folio = shmem_read_folio(mapping, idx); > + from_folio = shmem_read_folio_gfp(mapping, idx, mapping_gfp_mask(mapping) > + | additional_gfp); > if (IS_ERR(from_folio)) > return PTR_ERR(from_folio); > > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c > index 8fa9e09f6ee5..aa41099c5ecf 100644 > --- a/drivers/gpu/drm/ttm/ttm_pool.c > +++ b/drivers/gpu/drm/ttm/ttm_pool.c > @@ -530,6 +530,8 @@ static int ttm_pool_restore_commit(struct ttm_pool_tt_restore *restore, > p = first_page[i]; > if (ttm_backup_page_ptr_is_handle(p)) { > unsigned long handle = ttm_backup_page_ptr_to_handle(p); > + gfp_t additional_gfp = ctx->gfp_retry_mayfail ? > + __GFP_RETRY_MAYFAIL | __GFP_NOWARN : 0; > > if (IS_ENABLED(CONFIG_FAULT_INJECTION) && ctx->interruptible && > should_fail(&backup_fault_inject, 1)) { > @@ -543,7 +545,8 @@ static int ttm_pool_restore_commit(struct ttm_pool_tt_restore *restore, > } > > ret = ttm_backup_copy_page(backup, restore->alloced_page + i, > - handle, ctx->interruptible); > + handle, ctx->interruptible, > + additional_gfp); > if (ret) > break; > > diff --git a/include/drm/ttm/ttm_backup.h b/include/drm/ttm/ttm_backup.h > index c33cba111171..29b9c855af77 100644 > --- a/include/drm/ttm/ttm_backup.h > +++ b/include/drm/ttm/ttm_backup.h > @@ -56,7 +56,7 @@ ttm_backup_page_ptr_to_handle(const struct page *page) > void ttm_backup_drop(struct file *backup, pgoff_t handle); > > int ttm_backup_copy_page(struct file *backup, struct page *dst, > - pgoff_t handle, bool intr); > + pgoff_t handle, bool intr, gfp_t additional_gfp); > > s64 > ttm_backup_backup_page(struct file *backup, struct page *page, Reviewed-by: Maarten Lankhorst <dev@lankhorst.se> Kind regards, Maarten Lankhorst ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-03-10 14:11 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-27 16:00 [PATCH 0/2] drm/ttm: Improve the TTM operation context gfp_retry_mayfail behaviour Thomas Hellström 2026-02-27 16:00 ` [PATCH 1/2] drm/ttm: Don't spam the log on buffer object backing store allocation failure Thomas Hellström 2026-02-27 21:01 ` Matthew Brost 2026-03-02 9:02 ` Christian König 2026-03-02 9:39 ` Thomas Hellström 2026-03-09 9:36 ` Simona Vetter 2026-02-27 16:00 ` [PATCH 2/2] drm/ttm: Avoid invoking the OOM killer when reading back swapped content Thomas Hellström 2026-03-10 14:10 ` Maarten Lankhorst
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox