On Mon, 2026-07-13 at 09:58 +0000, Krzysztof Karas wrote:
> Continue unloading shmem_sg_alloc_table by placing reading
> folios and shrink call into a new helper.
> Make the loop a bit more reader-friendly by removing iteration
> over a structure and replacing it with a do-while loop.
>
> Signed-off-by: Krzysztof Karas <[email protected]>
> ---
> v3:
> * Split refactoring and put it after the fix in shmem folio
> counting suggested by Andi.
> * Use do-while loop suggested by Robin.
>
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 102 ++++++++++++----------
> 1 file changed, 55 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 4a61b012fb6f..7c8de8fe0a22 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -78,6 +78,55 @@ static int validate_size(size_t size, unsigned int
> page_count,
> return 0;
> }
>
> +static struct folio *shmem_shrink_get_folio(struct address_space *mapping,
> + unsigned long folio_index,
> + gfp_t gfp, unsigned int page_count,
> + struct drm_i915_private *i915)
> +{
> + struct folio *folio = NULL;
> + unsigned int retries = 2;
> +
> + do {
> + cond_resched();
> + folio = shmem_read_folio_gfp(mapping, folio_index, gfp);
> + if (IS_ERR(folio)) {
Going again through then unused shrinking and modification of gfp doesn't
make sense, I believe. Could be avoided based on retries value as an
additional condition.
Thanks,
Janusz
> + i915_gem_shrink(NULL, i915, 2 * page_count, NULL,
> + I915_SHRINK_BOUND |
> I915_SHRINK_UNBOUND);
> +
> + /*
> + * We've tried hard to allocate the memory by reaping
> + * our own buffer, now let the real VM do its job and
> + * go down in flames if truly OOM.
> + *
> + * However, since graphics tend to be disposable,
> + * defer the oom here by reporting the ENOMEM back
> + * to userspace.
> + *
> + * Reclaim and warn, but no oom.
> + */
> + gfp = mapping_gfp_mask(mapping);
> +
> + /*
> + * Our bo are always dirty and so we require
> + * kswapd to reclaim our pages (direct reclaim
> + * does not effectively begin pageout of our
> + * buffers on its own). However, direct reclaim
> + * only waits for kswapd when under allocation
> + * congestion. So as a result __GFP_RECLAIM is
> + * unreliable and fails to actually reclaim our
> + * dirty pages -- unless you try over and over
> + * again with !__GFP_NORETRY. However, we still
> + * want to fail this allocation rather than
> + * trigger the out-of-memory killer and for
> + * this we want __GFP_RETRY_MAYFAIL.
> + */
> + gfp |= __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
> + }
> + } while (IS_ERR(folio) && --retries);
> +
> + return folio;
> +}
> +
> int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
> size_t size, struct intel_memory_region *mr,
> struct address_space *mapping,
> @@ -117,57 +166,16 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
> struct sg_table *st,
> for (i = 0; i < page_count; i++) {
> unsigned long folio_page_index = 0;
> unsigned long nr_pages;
> - const unsigned int shrink[] = {
> - I915_SHRINK_BOUND | I915_SHRINK_UNBOUND,
> - 0,
> - }, *s = shrink;
> gfp_t gfp = noreclaim;
>
> /* Grab the next folio if we exhausted the current one. */
> if (!i || i > folio_end) {
> - do {
> - cond_resched();
> - folio = shmem_read_folio_gfp(mapping, i, gfp);
> - if (!IS_ERR(folio))
> - break;
> -
> - if (!*s) {
> - ret = PTR_ERR(folio);
> - goto err_sg;
> - }
> -
> - i915_gem_shrink(NULL, i915, 2 * page_count,
> NULL, *s++);
> -
> - /*
> - * We've tried hard to allocate the memory by
> reaping
> - * our own buffer, now let the real VM do its
> job and
> - * go down in flames if truly OOM.
> - *
> - * However, since graphics tend to be disposable,
> - * defer the oom here by reporting the ENOMEM
> back
> - * to userspace.
> - */
> - if (!*s) {
> - /* reclaim and warn, but no oom */
> - gfp = mapping_gfp_mask(mapping);
> -
> - /*
> - * Our bo are always dirty and so we
> require
> - * kswapd to reclaim our pages (direct
> reclaim
> - * does not effectively begin pageout
> of our
> - * buffers on its own). However, direct
> reclaim
> - * only waits for kswapd when under
> allocation
> - * congestion. So as a result
> __GFP_RECLAIM is
> - * unreliable and fails to actually
> reclaim our
> - * dirty pages -- unless you try over
> and over
> - * again with !__GFP_NORETRY. However,
> we still
> - * want to fail this allocation rather
> than
> - * trigger the out-of-memory killer and
> for
> - * this we want __GFP_RETRY_MAYFAIL.
> - */
> - gfp |= __GFP_RETRY_MAYFAIL |
> __GFP_NOWARN;
> - }
> - } while (1);
> + folio = shmem_shrink_get_folio(mapping, i, gfp,
> + page_count, i915);
> + if (IS_ERR(folio)) {
> + ret = PTR_ERR(folio);
> + goto err_sg;
> + }
>
> folio_start = folio_pgoff(folio);
> folio_end = folio_start + folio_nr_pages(folio) - 1;
