Before addition of commit 029ae067431a
("drm/i915: Fix potential overflow of shmem scatterlist length")
and after folios were introduced complete folios were always
allocated, possibly overloading the scatterlist capacity which
was never truly limited to PAGE_SIZE when requested via
max_segment. The above commit addressed scatterlist overloading,
but unintentionally disabled PAGE_SIZE as a valid max_segment
value and failed to take care of remaining pages from folios
above max_segment boundary.
This created a state, where multitude of scatterlists were used
for the same folio, but never counting enough of its pages to
jump to the next folio.
Track how many pages have already been counted in a folio and
use that number as an offset on consecutive allocations from the
same folio to ensure it is fully covered before reading next
folio.
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 <[email protected]>
---
v4:
* Rewrote commit message to be more explicit about the problem
(Janusz);
* Moved max_segment validation to the beginning of the function
(Janusz);
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 123 ++++++++++++++--------
1 file changed, 77 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 06543ae60706..af195db63038 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -68,10 +68,13 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
unsigned int max_segment)
{
unsigned int page_count; /* restricted by sg_alloc_table */
- unsigned long i;
+ unsigned long next_pfn = 0; /* suppress gcc warning */
+ unsigned long folio_start = 0;
+ unsigned long folio_end = 0;
+ struct folio *folio = NULL;
struct scatterlist *sg;
- unsigned long next_pfn = 0; /* suppress gcc warning */
gfp_t noreclaim;
+ unsigned long i;
int ret;
if (overflows_type(size / PAGE_SIZE, page_count))
@@ -88,6 +91,9 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
if (sg_alloc_table(st, page_count, GFP_KERNEL | __GFP_NOWARN))
return -ENOMEM;
+ if (max_segment < PAGE_SIZE)
+ return -EINVAL;
+
/*
* Get the list of pages out of our struct file. They'll be pinned
* at this point until we release them.
@@ -101,7 +107,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
sg = st->sgl;
st->nents = 0;
for (i = 0; i < page_count; i++) {
- struct folio *folio;
+ unsigned long folio_page_index = 0;
unsigned long nr_pages;
const unsigned int shrink[] = {
I915_SHRINK_BOUND | I915_SHRINK_UNBOUND,
@@ -109,71 +115,95 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
}, *s = shrink;
gfp_t gfp = noreclaim;
- do {
- cond_resched();
- folio = shmem_read_folio_gfp(mapping, i, gfp);
- if (!IS_ERR(folio))
- break;
+ /* 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;
- }
+ 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);
+ i915_gem_shrink(NULL, i915, 2 * page_count,
NULL, *s++);
/*
- * 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);
+ * 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_start = folio_pgoff(folio);
+ folio_end = folio_start + folio_nr_pages(folio) - 1;
+ }
+
+ folio_page_index = i - folio_start;
+ if (WARN_ON_ONCE(folio_page_index >= folio_nr_pages(folio))) {
+ ret = -EINVAL;
+ folio_put(folio);
+ goto err_sg;
+ }
nr_pages = min_array(((unsigned long[]) {
- folio_nr_pages(folio),
+ folio_nr_pages(folio) -
folio_page_index,
page_count - i,
max_segment / PAGE_SIZE,
}), 3);
if (!i ||
sg->length >= max_segment ||
- folio_pfn(folio) != next_pfn) {
+ folio_pfn(folio) + folio_page_index != next_pfn) {
if (i)
sg = sg_next(sg);
st->nents++;
- sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0);
+ sg_set_page(sg, folio_page(folio, folio_page_index),
+ nr_pages * PAGE_SIZE, 0);
} else {
+ /*
+ * If our prediction about folio placement is true and
+ * scatterlist still has space left for more pages,
+ * then we land here.
+ */
nr_pages = min_t(unsigned long, nr_pages,
(max_segment - sg->length) /
PAGE_SIZE);
sg->length += nr_pages * PAGE_SIZE;
}
- next_pfn = folio_pfn(folio) + nr_pages;
+
+ /*
+ * We assume folios are placed one after the other in memory
+ * and predict where the next folio begins.
+ */
+ next_pfn = folio_pfn(folio) + folio_page_index + nr_pages;
i += nr_pages - 1;
/* Check that the i965g/gm workaround works. */
@@ -186,6 +216,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
i915_sg_trim(st);
return 0;
+
err_sg:
sg_mark_end(sg);
if (sg != st->sgl) {
--
2.34.1
