[RFC 0/2] Improve memory management for large object allocations when i915/shmem is used with iommu

[RFC 0/2] Improve memory management for large object allocations when i915/shmem is used with iommu

[Krzysztof Karas]

It was observed that allocating large objects via i915 driver
(igt-gpu-tools/tests/gem_exec_big/single) the folios and their
pages were not handled properly leading to buffer corruptions
during relocations.
Furthermore, using iommu driver in this context would leave
residual mappings in memory that could not be released, hogging
available RAM even after the process ended.

Krzysztof Karas (2):
  drm/i915/shmem: Prevent overflows on small segments
  drivers/iommu: Unroll unsuccessful mapping

 drivers/gpu/drm/i915/gem/i915_gem_shmem.c |  2 +-
 drivers/iommu/dma-iommu.c                 | 10 ++++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.34.1

[RFC 1/2] drm/i915/shmem: Prevent overflows on small segments

[Krzysztof Karas]

With addition of commit 029ae067431a
("drm/i915: Fix potential overflow of shmem scatterlist length")
max_segment size was included in calculating a number of pages
for the scatterlist. This meant that segment sizes considerably
smaller than number of pages in a folio [1], were not enough to
jump to the next folio. In result, sg_set_folio() was called
multiple times with nr_pages smaller than folio size, using
many scatterlists, all pointing to the beginning pages of the
folio and never fully covering its range of pages and corrupting
mappings.

[1] See shmem_get_pages(), where segment size is set to
PAGE_SIZE.

Suggested-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Fixes: 029ae067431a ("drm/i915: Fix potential overflow of shmem scatterlist length")
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/15816
Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 06543ae60706..ac9b263c341a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -156,7 +156,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
 		nr_pages = min_array(((unsigned long[]) {
 					folio_nr_pages(folio),
 					page_count - i,
-					max_segment / PAGE_SIZE,
+					i915_sg_segment_size(i915->drm.dev) / PAGE_SIZE,
 				      }), 3);
 
 		if (!i ||
-- 
2.34.1

[RFC 2/2] drivers/iommu: Unroll unsuccessful mapping

[Krzysztof Karas]

Currently, if iommu maps fewer bytes than requested (iova_len),
it proceeds to free the iova, but never tries to unmap already
touched bytes. This behavior may cause memory hogging down the
line.

Correct that by unmapping before exiting.

Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
 drivers/iommu/dma-iommu.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 54d96e847f16..64ac2bdfc574 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1515,8 +1515,14 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 	 * implementation - it knows better than we do.
 	 */
 	ret = iommu_map_sg(domain, iova, sg, nents, prot, GFP_ATOMIC);
-	if (ret < 0 || ret < iova_len)
+	if (ret < 0 || ret < iova_len) {
+		if (ret > 0) {
+			/* Unmap partially mapped bytes before freeing IOVA */
+			if (iommu_unmap(domain, iova, ret) != ret)
+				ret = -EIO;
+		}
 		goto out_free_iova;
+	}
 
 	return __finalise_sg(dev, sg, nents, iova);
 
@@ -1525,7 +1531,7 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
 out_restore_sg:
 	__invalidate_sg(sg, nents);
 out:
-	if (ret != -ENOMEM && ret != -EREMOTEIO)
+	if (ret != -ENOMEM && ret != -EREMOTEIO && ret != -EIO)
 		return -EINVAL;
 	return ret;
 }
-- 
2.34.1

Re: [RFC 1/2] drm/i915/shmem: Prevent overflows on small segments

[Janusz Krzysztofik]

Hi Krzysztof,

On Wed, 2026-05-27 at 14:08 +0000, Krzysztof Karas wrote:
> With addition of commit 029ae067431a
> ("drm/i915: Fix potential overflow of shmem scatterlist length")
> max_segment size was included in calculating a number of pages
> for the scatterlist. This meant that segment sizes considerably
> smaller than number of pages in a folio [1], were not enough to
> jump to the next folio.

I think that scope of the issue you are addressing was limited specifically
to max_segment == PAGE_SIZE, passed from shmem_get_pages() to
shmem_sg_alloc_table() after first attempt with max_segment ==
i915_sg_segment_size() failed.  Then, if that's the case and I'm not wrong,
your "considerably smaller" wording is not strict enough for me.

Other than that, I think that this change correctly fixes the bug that I
introduced with the blamed patch (sorry).

When resubmitting, please add dri-devel@lists.freedesktop.org to Cc:,
that's mandatory for i915 patches.

Thanks,
Janusz


> In result, sg_set_folio() was called
> multiple times with nr_pages smaller than folio size, using
> many scatterlists, all pointing to the beginning pages of the
> folio and never fully covering its range of pages and corrupting
> mappings.
> 
> [1] See shmem_get_pages(), where segment size is set to
> PAGE_SIZE.
> 
> Suggested-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Fixes: 029ae067431a ("drm/i915: Fix potential overflow of shmem scatterlist length")
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/15816
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 06543ae60706..ac9b263c341a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -156,7 +156,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
>  		nr_pages = min_array(((unsigned long[]) {
>  					folio_nr_pages(folio),
>  					page_count - i,
> -					max_segment / PAGE_SIZE,
> +					i915_sg_segment_size(i915->drm.dev) / PAGE_SIZE,
>  				      }), 3);
>  
>  		if (!i ||

Re: [RFC 2/2] drivers/iommu: Unroll unsuccessful mapping

[Janusz Krzysztofik]

Hi Krzysztof,

On Wed, 2026-05-27 at 14:08 +0000, Krzysztof Karas wrote:
> Currently, if iommu maps fewer bytes than requested (iova_len),
> it proceeds to free the iova, but never tries to unmap already
> touched bytes. This behavior may cause memory hogging down the
> line.
> 
> Correct that by unmapping before exiting.
> 
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
>  drivers/iommu/dma-iommu.c | 10 ++++++++--

You have to submit this patch to IOMMU DMA-API LAYER maintainers (see
MAINTAINERS file for list and maintainer addresses) in order to get binding
comments, I believe.

Thanks,
Janusz

>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 54d96e847f16..64ac2bdfc574 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1515,8 +1515,14 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
>  	 * implementation - it knows better than we do.
>  	 */
>  	ret = iommu_map_sg(domain, iova, sg, nents, prot, GFP_ATOMIC);
> -	if (ret < 0 || ret < iova_len)
> +	if (ret < 0 || ret < iova_len) {
> +		if (ret > 0) {
> +			/* Unmap partially mapped bytes before freeing IOVA */
> +			if (iommu_unmap(domain, iova, ret) != ret)
> +				ret = -EIO;
> +		}
>  		goto out_free_iova;
> +	}
>  
>  	return __finalise_sg(dev, sg, nents, iova);
>  
> @@ -1525,7 +1531,7 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
>  out_restore_sg:
>  	__invalidate_sg(sg, nents);
>  out:
> -	if (ret != -ENOMEM && ret != -EREMOTEIO)
> +	if (ret != -ENOMEM && ret != -EREMOTEIO && ret != -EIO)
>  		return -EINVAL;
>  	return ret;
>  }

✗ i915.CI.BAT: failure for Improve memory management for large object allocations when i915/shmem is used with iommu

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

== Series Details ==

Series: Improve memory management for large object allocations when i915/shmem is used with iommu
URL   : https://patchwork.freedesktop.org/series/167389/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_18561 -> Patchwork_167389v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_167389v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_167389v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_167389v1/index.html

Participating hosts (42 -> 39)
------------------------------

  Missing    (3): bat-dg2-13 fi-snb-2520m bat-adls-6 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_167389v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@vma:
    - bat-arlh-2:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18561/bat-arlh-2/igt@i915_selftest@live@vma.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_167389v1/bat-arlh-2/igt@i915_selftest@live@vma.html

  
Known issues
------------

  Here are the changes found in Patchwork_167389v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - bat-arlh-2:         [PASS][3] -> [INCOMPLETE][4] ([i915#15978])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18561/bat-arlh-2/igt@i915_selftest@live.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_167389v1/bat-arlh-2/igt@i915_selftest@live.html

  
  [i915#15978]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15978


Build changes
-------------

  * Linux: CI_DRM_18561 -> Patchwork_167389v1

  CI-20190529: 20190529
  CI_DRM_18561: 5390f2273d45bb259d88508828018c0fbbb79d32 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8938: b024a3b67372962ff6e643d3998c5cf5acc07081 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_167389v1: 5390f2273d45bb259d88508828018c0fbbb79d32 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_167389v1/index.html

[-- Attachment #2: Type: text/html, Size: 3109 bytes --]

Re: [RFC 1/2] drm/i915/shmem: Prevent overflows on small segments

[Sebastian Brzezinka]

Hi Krzysztof,

On Wed May 27, 2026 at 4:08 PM CEST, Krzysztof Karas wrote:
> With addition of commit 029ae067431a
> ("drm/i915: Fix potential overflow of shmem scatterlist length")
> max_segment size was included in calculating a number of pages
> for the scatterlist. This meant that segment sizes considerably
> smaller than number of pages in a folio [1], were not enough to
> jump to the next folio. In result, sg_set_folio() was called
> multiple times with nr_pages smaller than folio size, using
> many scatterlists, all pointing to the beginning pages of the
> folio and never fully covering its range of pages and corrupting
> mappings.
>
> [1] See shmem_get_pages(), where segment size is set to
> PAGE_SIZE.
>
> Suggested-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Fixes: 029ae067431a ("drm/i915: Fix potential overflow of shmem scatterlist length")
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/15816
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 06543ae60706..ac9b263c341a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -156,7 +156,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
>  		nr_pages = min_array(((unsigned long[]) {
>  					folio_nr_pages(folio),
>  					page_count - i,
> -					max_segment / PAGE_SIZE,
> +					i915_sg_segment_size(i915->drm.dev) / PAGE_SIZE,
I don't think we can use i915_sg_segment_size() here, please correct me
if I'm wrong.

When DMA mapping fails, shmem_get_pages() sets max_segment = PAGE_SIZE
and retries. If we replace max_segment with i915_sg_segment_size() in the
min_array, nothing really changes between the first and second attempt,
we end up computing the same nr_pages as before and building the same
sg table that just failed to map.

It also breaks the sg->length check on line 163. After the first folio,
sg->length equals i915_sg_segment_size(). With max_segment=PAGE_SIZE the
check sg->length >= max_segment is always true, never and the PAGE_SIZE
retry has no effect.

-- 
Best regards,
Sebastian

Re: [RFC 1/2] drm/i915/shmem: Prevent overflows on small segments

[Janusz Krzysztofik]

Hi Sebastian,

On Thu, 2026-05-28 at 15:55 +0200, Sebastian Brzezinka wrote:
> Hi Krzysztof,
> 
> On Wed May 27, 2026 at 4:08 PM CEST, Krzysztof Karas wrote:
> > With addition of commit 029ae067431a
> > ("drm/i915: Fix potential overflow of shmem scatterlist length")
> > max_segment size was included in calculating a number of pages
> > for the scatterlist. This meant that segment sizes considerably
> > smaller than number of pages in a folio [1], were not enough to
> > jump to the next folio. In result, sg_set_folio() was called
> > multiple times with nr_pages smaller than folio size, using
> > many scatterlists, all pointing to the beginning pages of the
> > folio and never fully covering its range of pages and corrupting
> > mappings.
> > 
> > [1] See shmem_get_pages(), where segment size is set to
> > PAGE_SIZE.
> > 
> > Suggested-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > Fixes: 029ae067431a ("drm/i915: Fix potential overflow of shmem scatterlist length")
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/15816
> > Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > index 06543ae60706..ac9b263c341a 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > @@ -156,7 +156,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
> >  		nr_pages = min_array(((unsigned long[]) {
> >  					folio_nr_pages(folio),
> >  					page_count - i,
> > -					max_segment / PAGE_SIZE,
> > +					i915_sg_segment_size(i915->drm.dev) / PAGE_SIZE,
> I don't think we can use i915_sg_segment_size() here, please correct me
> if I'm wrong.

Looking at this again, I think you are right to some extent, and
unfortunately my suggestion to use i915_sg_segment_size() here wasn't based
on sufficiently deep analysis of the issue Krzysztof was trying to address.

While my commit 029ae067431a ("drm/i915: Fix potential overflow of shmem
scatterlist length") suggested that clamping nr_pages also to max_segment
was needed only to avoid the overflow, in fact it also changed the way
pages were allocated when current scatterlist length exceeded max_segment
and a new scatterlist was allocated.  Assuming the intention of calling
shmem_sg_alloc_table() again with max_segment == PAGE_SIZE once it failed
when called with max_segment == i915_sg_segment_size() was to allocate
pages one per scatterlist, that rule was never followed after commit
0b62af28f249b ("i915: convert shmem_sg_free_table() to use a folio_batch")
and newly allocated scatterlists were still populated with nr_pages not
clamped to max_segment == PAGE_SIZE.  With my change addressing the
potential overflow, number of pages allocated to a single scatterlist is
now always clamped to max_segment, and one page per scatterlist mode should
now work as designed.

That said, I think we need to revisit the issue identified by Krzysztof and
try to find its root cause again.  Maybe we should also revisit either the
idea or implementation of allocating one page per scatterlist in case when
allocating larger chunks failed, because that seems to be strictly related
to what Krzysztof describes as problematic in his commit description ("...
sg_set_folio() was called multiple times with nr_pages smaller than folio
size, using many scatterlists, all pointing to the beginning pages of the
folio and never fully covering its range of pages ...").

Thanks,
Janusz

> 
> When DMA mapping fails, shmem_get_pages() sets max_segment = PAGE_SIZE
> and retries. If we replace max_segment with i915_sg_segment_size() in the
> min_array, nothing really changes between the first and second attempt,
> we end up computing the same nr_pages as before and building the same
> sg table that just failed to map.
> 
> It also breaks the sg->length check on line 163. After the first folio,
> sg->length equals i915_sg_segment_size(). With max_segment=PAGE_SIZE the
> check sg->length >= max_segment is always true, never and the PAGE_SIZE
> retry has no effect.

Re: [RFC 1/2] drm/i915/shmem: Prevent overflows on small segments

[Krzysztof Karas]

Hi Janusz,

> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > > @@ -156,7 +156,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
> > >  		nr_pages = min_array(((unsigned long[]) {
> > >  					folio_nr_pages(folio),
> > >  					page_count - i,
> > > -					max_segment / PAGE_SIZE,
> > > +					i915_sg_segment_size(i915->drm.dev) / PAGE_SIZE,
> > I don't think we can use i915_sg_segment_size() here, please correct me
> > if I'm wrong.
> 

[part of the comment cut here]

> That said, I think we need to revisit the issue identified by Krzysztof and
> try to find its root cause again.  Maybe we should also revisit either the
> idea or implementation of allocating one page per scatterlist in case when
> allocating larger chunks failed, because that seems to be strictly related
> to what Krzysztof describes as problematic in his commit description ("...
> sg_set_folio() was called multiple times with nr_pages smaller than folio
> size, using many scatterlists, all pointing to the beginning pages of the
> folio and never fully covering its range of pages ...").
I wanted to avoid re-implementing the hard to read parts of the
code my changes touch in this series, but since it turns out to
be bigger deal than I thought, I may do that as well.

-- 
Best Regards,
Krzysztof