Hi,
On 28/07/2023 02:58, Jiasheng Jiang wrote:
Add the check for the return value of bitmap_zalloc() in order to
guarantee the success of the allocation.
Fixes: e9b73c67390a ("drm/i915: Reduce memory pressure during shrinker by
preallocating swizzle pages")
Signed-off-by: Jiasheng Jiang <[email protected]>
---
drivers/gpu/drm/i915/gem/i915_gem_tiling.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index a049ca0b7980..e9cf99d95966 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -311,6 +311,11 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
if (!obj->bit_17) {
obj->bit_17 = bitmap_zalloc(obj->base.size >>
PAGE_SHIFT,
GFP_KERNEL);
+ if (!obj->bit_17) {
+ i915_gem_object_unlock(obj);
+ i915_gem_object_release_mmap_gtt(obj);
+ return -ENOMEM;
+ }
Hm the comment few lines above says:
/* Try to preallocate memory required to save swizzling on put-pages */
Lets emphasis the *try* for now. Then once the obj->bit_17 is attempted to be
used we have this:
i915_gem_object_save_bit_17_swizzle(..)
{
...
if (obj->bit_17 == NULL) {
obj->bit_17 = bitmap_zalloc(page_count, GFP_KERNEL);
if (obj->bit_17 == NULL) {
drm_err(obj->base.dev,
"Failed to allocate memory for bit 17
record\n");
return;
}
}
So despite this area of the driver being a bit before my time, I'd say it quite
possibly works as designed - only *tries* to preallocate but does not have to
and can cope with a later failure.
Good question might be why wouldn't it be better to do what you suggest. Trade
off would be between failing the ioctl and possibly crashing the application,
versus visual corruption if at use time allocation fails.
The whole swizzling thing also only applies to old GPUs, stuff before
Broadwell, which itself was released in 2014. So it is tempting to err on the
side of caution and leave it as is. I'll mull it over in the background, or
maybe someone else will have an opinion too.
Regards,
Tvrtko
}
} else {
bitmap_free(obj->bit_17);
