shmem_sg_alloc_table is a complex and hard to read function. Split its logic into smaller pieces to improve readability and reduce indentation. Change main "for" loop into "while" to get rid of obscure iterator "i" and be more explicit in traversing scatterlist.
Signed-off-by: Krzysztof Karas <[email protected]> --- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 176 ++++++++++++---------- 1 file changed, 100 insertions(+), 76 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..b5ae7e5f80a0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -62,22 +62,12 @@ void shmem_sg_free_table(struct sg_table *st, struct address_space *mapping, sg_free_table(st); } -int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st, - size_t size, struct intel_memory_region *mr, - struct address_space *mapping, - unsigned int max_segment) +static int validate_size(size_t size, unsigned int page_count, + struct intel_memory_region *mr) { - unsigned int page_count; /* restricted by sg_alloc_table */ - unsigned long i; - struct scatterlist *sg; - unsigned long next_pfn = 0; /* suppress gcc warning */ - gfp_t noreclaim; - int ret; - if (overflows_type(size / PAGE_SIZE, page_count)) return -E2BIG; - page_count = size / PAGE_SIZE; /* * If there's no chance of allocating enough pages for the whole * object, bail early. @@ -85,7 +75,81 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st, if (size > resource_size(&mr->region)) return -ENOMEM; - if (sg_alloc_table(st, page_count, GFP_KERNEL | __GFP_NOWARN)) + return 0; +} + +static struct folio *shmem_shrink_get_folio(struct address_space *mapping, + unsigned long folio_index, + gfp_t gfp, unsigned int pages_left, + struct drm_i915_private *i915) +{ +#define MAX_READS 2 + struct folio *folio; + unsigned int i; + + for (i = 0; i < MAX_READS; i++) { + cond_resched(); + folio = shmem_read_folio_gfp(mapping, folio_index, gfp); + if (!IS_ERR(folio) || i == MAX_READS - 1) + return folio; + + i915_gem_shrink(NULL, i915, 2 * pages_left, NULL, + I915_SHRINK_BOUND | I915_SHRINK_UNBOUND); + + /* + * 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. + * + * 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; + } + + /* Should never happen */ + WARN_ON_ONCE(1); + return ERR_PTR(-EINVAL); +} + +int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st, + size_t size, struct intel_memory_region *mr, + struct address_space *mapping, + unsigned int max_segment) +{ + unsigned int pages_left; /* restricted by sg_alloc_table */ + unsigned long next_pfn = 0; /* suppress gcc warning */ + unsigned long pages_done = 0; + struct scatterlist *sg; + gfp_t noreclaim; + int ret; + + pages_left = size / PAGE_SIZE; + + ret = validate_size(size, pages_left, mr); + if (ret < 0) + return ret; + + if (sg_alloc_table(st, pages_left, GFP_KERNEL | __GFP_NOWARN)) return -ENOMEM; /* @@ -98,73 +162,32 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st, noreclaim = mapping_gfp_constraint(mapping, ~__GFP_RECLAIM); noreclaim |= __GFP_NORETRY | __GFP_NOWARN; - sg = st->sgl; st->nents = 0; - for (i = 0; i < page_count; i++) { - struct folio *folio; + sg = st->sgl; + + while (pages_left) { unsigned long nr_pages; - const unsigned int shrink[] = { - I915_SHRINK_BOUND | I915_SHRINK_UNBOUND, - 0, - }, *s = shrink; gfp_t gfp = noreclaim; + struct folio *folio; - 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); - - /* - * 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); - - nr_pages = min_array(((unsigned long[]) { - folio_nr_pages(folio), - page_count - i, - max_segment / PAGE_SIZE, - }), 3); - - if (!i || - sg->length >= max_segment || - folio_pfn(folio) != next_pfn) { - if (i) - sg = sg_next(sg); + folio = shmem_shrink_get_folio(mapping, pages_done, gfp, + pages_left, i915); + if (IS_ERR(folio)) { + ret = PTR_ERR(folio); + goto err_sg; + } + nr_pages = min_array(((unsigned long[]){ + folio_nr_pages(folio), + pages_left, + max_segment / PAGE_SIZE, + }), 3); + if (!st->nents) { + st->nents++; + sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0); + } else if (sg->length >= max_segment || + folio_pfn(folio) != next_pfn) { + sg = sg_next(sg); st->nents++; sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0); } else { @@ -174,7 +197,8 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st, sg->length += nr_pages * PAGE_SIZE; } next_pfn = folio_pfn(folio) + nr_pages; - i += nr_pages - 1; + pages_done += nr_pages; + pages_left -= nr_pages; /* Check that the i965g/gm workaround works. */ GEM_BUG_ON(gfp & __GFP_DMA32 && next_pfn >= 0x00100000UL); -- 2.34.1
