Am 19.03.21 um 20:06 schrieb Daniel Vetter:
On Fri, Mar 19, 2021 at 07:53:48PM +0100, Christian König wrote:
Am 19.03.21 um 18:52 schrieb Daniel Vetter:
On Fri, Mar 19, 2021 at 03:08:57PM +0100, Christian König wrote:
Don't print a warning when we fail to allocate a page for swapping things out.
Also rely on memalloc_nofs_save/memalloc_nofs_restore instead of GFP_NOFS.
Uh this part doesn't make sense. Especially since you only do it for the
debugfs file, not in general. Which means you've just completely broken
the shrinker.
Are you sure? My impression is that GFP_NOFS should now work much more out
of the box with the memalloc_nofs_save()/memalloc_nofs_restore().
Yeah, if you'd put it in the right place :-)
But also -mm folks are very clear that memalloc_no*() family is for dire
situation where there's really no other way out. For anything where you
know what you're doing, you really should use explicit gfp flags.
My impression is just the other way around. You should try to avoid the
NOFS/NOIO flags and use the memalloc_no* approach instead.
If this is just to paper over the seq_printf doing the wrong allocations,
then just move that out from under the fs_reclaim_acquire/release part.
No, that wasn't the problem.
We have just seen to many failures to allocate pages for swapout and I think
that would improve this because in a lot of cases we can then immediately
swap things out instead of having to rely on upper layers.
Yeah, you broke it. Now the real shrinker is running with GFP_KERNEL,
because your memalloc_no is only around the debugfs function. And ofc it's
much easier to allocate with GFP_KERNEL, right until you deadlock :-)
The problem here is that for example kswapd calls the shrinker without
holding a FS lock as far as I can see.
And it is rather sad that we can't optimize this case directly.
Anyway you are right if some caller doesn't use the memalloc_no*()
approach we are busted.
Going to change the patch to only not warn for the moment.
Regards,
Christian.
Shrinking is hard, there's no easy way out here.
Cheers, Daniel
Regards,
Christian.
__GFP_NOWARN should be there indeed I think.
-Daniel
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
drivers/gpu/drm/ttm/ttm_tt.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 2f0833c98d2c..86fa3e82dacc 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -369,7 +369,7 @@ static unsigned long ttm_tt_shrinker_scan(struct shrinker *shrink,
};
int ret;
- ret = ttm_bo_swapout(&ctx, GFP_NOFS);
+ ret = ttm_bo_swapout(&ctx, GFP_KERNEL | __GFP_NOWARN);
return ret < 0 ? SHRINK_EMPTY : ret;
}
@@ -389,10 +389,13 @@ static unsigned long ttm_tt_shrinker_count(struct shrinker *shrink,
static int ttm_tt_debugfs_shrink_show(struct seq_file *m, void *data)
{
struct shrink_control sc = { .gfp_mask = GFP_KERNEL };
+ unsigned int flags;
fs_reclaim_acquire(GFP_KERNEL);
+ flags = memalloc_nofs_save();
seq_printf(m, "%lu/%lu\n", ttm_tt_shrinker_count(&mm_shrinker, &sc),
ttm_tt_shrinker_scan(&mm_shrinker, &sc));
+ memalloc_nofs_restore(flags);
fs_reclaim_release(GFP_KERNEL);
return 0;
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
