[PATCH v1 1/1] drm/i915/gem: Use simpler versions of min*()

[PATCH v1 1/1] drm/i915/gem: Use simpler versions of min*()

[Andy Shevchenko]

Instead of harder to read min_array() and prone to mistakes min_t()
use min3() and min(). No functional changes intended. The types of
the values are unsigned, no side-effects are expected.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index ef9440166295..bf9e7480f0f5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -153,11 +153,8 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
 			}
 		} while (1);
 
-		nr_pages = min_array(((unsigned long[]) {
-					folio_nr_pages(folio),
-					page_count - i,
-					max_segment / PAGE_SIZE,
-				      }), 3);
+		nr_pages = min3(folio_nr_pages(folio),
+				page_count - i, max_segment / PAGE_SIZE);
 
 		if (!i ||
 		    sg->length >= max_segment ||
@@ -168,8 +165,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
 			st->nents++;
 			sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0);
 		} else {
-			nr_pages = min_t(unsigned long, nr_pages,
-					 (max_segment - sg->length) / PAGE_SIZE);
+			nr_pages = min(nr_pages, (max_segment - sg->length) / PAGE_SIZE);
 
 			sg->length += nr_pages * PAGE_SIZE;
 		}
-- 
2.50.1

Re: [PATCH v1 1/1] drm/i915/gem: Use simpler versions of min*()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Data corruption when splitting large folios into multiple scatterlist segments due to small max_segment size.
--

--- Patch [1]: [PATCH v1 1/1] drm/i915/gem: Use simpler versions of min*() ---
commit a3571cfa95156ba739dd7334aec0101cdc3aa63e
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

drm/i915/gem: Use simpler versions of min*()

This commit replaces the harder-to-read min_array() and min_t() 
with min3() and min() in shmem_sg_alloc_table(). No functional 
changes are intended.

> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -153,11 +153,8 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
>  			}
>  		} while (1);
>  
> -		nr_pages = min_array(((unsigned long[]) {
> -					folio_nr_pages(folio),
> -					page_count - i,
> -					max_segment / PAGE_SIZE,
> -				      }), 3);
> +		nr_pages = min3(folio_nr_pages(folio),
> +				page_count - i, max_segment / PAGE_SIZE);

[Severity: High]
This is a pre-existing issue, but does this correctly calculate the available
pages for a large folio?

It looks like folio_nr_pages(folio) uses the total folio size rather than the
remaining size from the current offset (e.g. folio_nr_pages(folio) - (i -
folio->index)).

>  
>  		if (!i ||
>  		    sg->length >= max_segment ||
> @@ -168,8 +165,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
>  			st->nents++;
>  			sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0);

[Severity: High]
This isn't a bug introduced by this patch, but can this hardcoded offset of 0
corrupt data when splitting large folios?

When a GEM object backed by shmem memory receives a large folio and the
IOMMU's max_segment is smaller than the large folio size, this restricts
nr_pages to the segment size. 

In the next iteration, i advances into the middle of the large folio, and
shmem_read_folio_gfp() returns the same head folio. The code evaluates
folio_pfn(folio) != next_pfn as true (since next_pfn advanced but
folio_pfn still points to the head) and calls sg_set_folio() again. 

Because the offset is hardcoded to 0, it appears the GPU will repeatedly map
the first max_segment bytes of the large folio rather than its subsequent
pages, causing the GPU and CPU memory views to become desynchronized.

>  		} else {
> -			nr_pages = min_t(unsigned long, nr_pages,
> -					 (max_segment - sg->length) / PAGE_SIZE);
> +			nr_pages = min(nr_pages, (max_segment - sg->length) / PAGE_SIZE);
>  
>  			sg->length += nr_pages * PAGE_SIZE;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825093552.4088094-1-andriy.shevchenko@linux.intel.com?part=1

Re: [PATCH v1 1/1] drm/i915/gem: Use simpler versions of min*()

[Andi Shyti]

Hi Andy,

On Tue, Aug 25, 2026 at 11:35:52AM +0200, Andy Shevchenko wrote:
> Instead of harder to read min_array() and prone to mistakes min_t()
> use min3() and min(). No functional changes intended. The types of
> the values are unsigned, no side-effects are expected.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index ef9440166295..bf9e7480f0f5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -153,11 +153,8 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
>  			}
>  		} while (1);
>  
> -		nr_pages = min_array(((unsigned long[]) {
> -					folio_nr_pages(folio),
> -					page_count - i,
> -					max_segment / PAGE_SIZE,
> -				      }), 3);
> +		nr_pages = min3(folio_nr_pages(folio),
> +				page_count - i, max_segment / PAGE_SIZE);

thanks for your patch, but there is another patch involving (and
fixing) this line of code[*]. You are welcome to join the review.

Thanks,
Andi

[*] https://patchwork.freedesktop.org/patch/746877/?series=172332&rev=1

>  
>  		if (!i ||
>  		    sg->length >= max_segment ||
> @@ -168,8 +165,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
>  			st->nents++;
>  			sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0);
>  		} else {
> -			nr_pages = min_t(unsigned long, nr_pages,
> -					 (max_segment - sg->length) / PAGE_SIZE);
> +			nr_pages = min(nr_pages, (max_segment - sg->length) / PAGE_SIZE);
>  
>  			sg->length += nr_pages * PAGE_SIZE;
>  		}
> -- 
> 2.50.1
> 

Re: [PATCH v1 1/1] drm/i915/gem: Use simpler versions of min*()

[Andy Shevchenko]

On Tue, Aug 25, 2026 at 03:44:40PM +0200, Andi Shyti wrote:
> On Tue, Aug 25, 2026 at 11:35:52AM +0200, Andy Shevchenko wrote:

...

> > -		nr_pages = min_array(((unsigned long[]) {
> > -					folio_nr_pages(folio),
> > -					page_count - i,
> > -					max_segment / PAGE_SIZE,
> > -				      }), 3);
> > +		nr_pages = min3(folio_nr_pages(folio),
> > +				page_count - i, max_segment / PAGE_SIZE);
> 
> thanks for your patch, but there is another patch involving (and
> fixing) this line of code[*]. You are welcome to join the review.

Thanks for pointing this out! Do you prefer my to be rebased on top?
Otherwise one can apply this patch first.

> [*] https://patchwork.freedesktop.org/patch/746877/?series=172332&rev=1

Note, my patch doesn't change existing behaviour.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] drm/i915/gem: Use simpler versions of min*()

[Andi Shyti]

Hi Andy,

> > > -		nr_pages = min_array(((unsigned long[]) {
> > > -					folio_nr_pages(folio),
> > > -					page_count - i,
> > > -					max_segment / PAGE_SIZE,
> > > -				      }), 3);
> > > +		nr_pages = min3(folio_nr_pages(folio),
> > > +				page_count - i, max_segment / PAGE_SIZE);
> > 
> > thanks for your patch, but there is another patch involving (and
> > fixing) this line of code[*]. You are welcome to join the review.
> 
> Thanks for pointing this out! Do you prefer my to be rebased on top?
> Otherwise one can apply this patch first.
> 
> > [*] https://patchwork.freedesktop.org/patch/746877/?series=172332&rev=1
> 
> Note, my patch doesn't change existing behaviour.

yeah... besides that patch has had some reviews to follow up;
Krzysztof will have a little conflict easy to fix.

I reviewed and pushed your branch to drm-intel-gt-next.

Thanks,
Andi