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.
Furthermore, current do-while loop goes over the same folio
multiple times, increasing the refcount each time, and breaks
get/put symmetry: many shmem_read_folio_gfp() gets and only one
put in i915_gem_object_put_pages_shmem() per 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")
Cc: [email protected]
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/15816
Signed-off-by: Krzysztof Karas <[email protected]>
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 101 +++++++++++++---------
1 file changed, 59 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index e35ee4bd456f..d57b5c67d47b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -71,6 +71,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
unsigned long i;
struct scatterlist *sg;
unsigned long next_pfn = 0; /* suppress gcc warning */
+ struct folio *folio = NULL;
gfp_t noreclaim;
int ret;
@@ -104,7 +105,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;
unsigned long nr_pages;
const unsigned int shrink[] = {
I915_SHRINK_BOUND | I915_SHRINK_UNBOUND,
@@ -112,51 +113,61 @@ 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;
-
- if (!*s) {
- ret = PTR_ERR(folio);
- goto err_sg;
- }
+ if (!i || (next_pfn >= folio_pfn(folio) +
folio_nr_pages(folio))) {
+ /* Grab the next folio if we exhausted the current one.
*/
+ do {
+ cond_resched();
+ folio = shmem_read_folio_gfp(mapping, i, gfp);
+ folio_page_index = 0;
+ if (!IS_ERR(folio))
+ break;
- i915_gem_shrink(NULL, i915, 2 * page_count, NULL, *s++);
+ if (!*s) {
+ ret = PTR_ERR(folio);
+ goto err_sg;
+ }
- /*
- * 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.
+ * 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.
*/
- gfp |= __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
- }
- } while (1);
+ 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);
+ } else {
+ /*
+ * Otherwise, we are still inside the same folio,
+ * so just update its page index.
+ */
+ folio_page_index = next_pfn - folio_pfn(folio);
+ }
- nr_pages = min3(folio_nr_pages(folio),
+ nr_pages = min3(folio_nr_pages(folio) - folio_page_index,
page_count - i, max_segment / PAGE_SIZE);
if (!i ||
@@ -166,13 +177,19 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
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 {
nr_pages = min(nr_pages, (max_segment - sg->length) /
PAGE_SIZE);
sg->length += nr_pages * PAGE_SIZE;
}
- next_pfn = folio_pfn(folio) + nr_pages;
+
+ /*
+ * Store the next PFN, so contiguous physical pages
+ * can be merged into the same SG entry.
+ */
+ next_pfn = folio_pfn(folio) + folio_page_index + nr_pages;
i += nr_pages - 1;
/* Check that the i965g/gm workaround works. */
--
2.34.1
