Hi Krzysztof,

On Wed, Jul 01, 2026 at 10:44:35AM +0000, Krzysztof Karas wrote:
> shmem_sg_alloc_table is a complex and hard to read function.
> Split its logic into smaller pieces to improve readability and
> reduce indentation. Change the main "for" loop into "while" to
> get rid of obscure iterator "i" and be more explicit in
> traversing scatterlist.

any chance we can split this cleanup into smaller pieces?

> Signed-off-by: Krzysztof Karas <[email protected]>

...

> +static struct folio *shmem_shrink_get_folio(struct address_space *mapping,
> +                                         unsigned long folio_index,
> +                                         gfp_t gfp, unsigned int pages_left,
> +                                         struct drm_i915_private *i915)
> +{
> +#define MAX_READS 2

This MAX_READS here is very ugly! Just use 2 and explain it in a
comment. In the 'if' below you can check out of "if (... || i)"
and still explain it in a comment.

> +     struct folio *folio;
> +     unsigned int i;
> +
> +     for (i = 0; i < MAX_READS; i++) {
> +             cond_resched();
> +             folio = shmem_read_folio_gfp(mapping, folio_index, gfp);
> +             if (!IS_ERR(folio) || i == MAX_READS - 1)
> +                     return folio;
> +
> +             i915_gem_shrink(NULL, i915, 2 * pages_left, NULL,

/pages_left/page_count/

> +                             I915_SHRINK_BOUND | I915_SHRINK_UNBOUND);
> +

...

> +     }
> +
> +     /* Should never happen */
> +     WARN_ON_ONCE(1);

no need.

Thanks,
Andi

> +     return ERR_PTR(-EINVAL);
> +}

Reply via email to