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 (see shmem_get_pages(),
rebuild_st label for context), were not enough to jump to the
next folio, which has never been a problem before folios have
been intoduced. In result, sg_set_folio() was called multiple
times with nr_pages smaller than folio size, using multitude of
scatterlists, all pointing to the beginning pages of the folio
and never fully covering its range of pages.
Track how many pages have already been counted in a 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]>
---
v3:
* Fixed a bug that caused first folio to never be considered.
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 120 +++++++++++++---------
1 file changed, 70 insertions(+), 50 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..0011d76f5b8c 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))
@@ -101,7 +104,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 +112,87 @@ 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;
- }
-
- 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);
+ /* 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;
+ }
+
+ 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,
+ max_t(unsigned int, 1, 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 {
nr_pages = min_t(unsigned long, nr_pages,
- (max_segment - sg->length) /
PAGE_SIZE);
+ max_t(unsigned long, 1,
+ (max_segment - sg->length) /
PAGE_SIZE));
sg->length += nr_pages * PAGE_SIZE;
}
- next_pfn = folio_pfn(folio) + nr_pages;
+
+ next_pfn = folio_pfn(folio) + folio_page_index + nr_pages;
i += nr_pages - 1;
/* Check that the i965g/gm workaround works. */
@@ -186,6 +205,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
