Skip to contentSkip to conversation

drm/amd/display: Use 2-factor allocator calls

PATCH In review Kees Cook 3 participants 4 messages Jul 4, 2018 – Jul 5, 2018

Subsystems (3)
DRM drivers Radeon and amdgpu DRM drivers AMD display core
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 drm/amd/display: Use 2-factor allocator calls

As already done treewide, switch from open-coded multiplication to 2-factor allocation helper.

Signed-off-by Kees Cook <[email protected]>
---
 drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/amd/display/modules/color/color_gamma.c +4 -4
index 98edaefa2b47..ee69c949bfbf 100644
@@ -1723,8 +1723,8 @@ bool  mod_color_calculate_curve(enum dc_transfer_func_predefined trans,
 		kvfree(rgb_regamma);
 	} else if (trans == TRANSFER_FUNCTION_HLG ||
 		trans == TRANSFER_FUNCTION_HLG12) {
-		rgb_regamma = kvzalloc(sizeof(*rgb_regamma) *
-				       (MAX_HW_POINTS + _EXTRA_POINTS),
+		rgb_regamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
+				       sizeof(*rgb_regamma),
 				       GFP_KERNEL);
 		if (!rgb_regamma)
 			goto rgb_regamma_alloc_fail;

@@ -1802,8 +1802,8 @@ bool  mod_color_calculate_degamma_curve(enum dc_transfer_func_predefined trans,
 		kvfree(rgb_degamma);
 	} else if (trans == TRANSFER_FUNCTION_HLG ||
 		trans == TRANSFER_FUNCTION_HLG12) {
-		rgb_degamma = kvzalloc(sizeof(*rgb_degamma) *
-				       (MAX_HW_POINTS + _EXTRA_POINTS),
+		rgb_degamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
+				       sizeof(*rgb_degamma),
 				       GFP_KERNEL);
 		if (!rgb_degamma)
 			goto rgb_degamma_alloc_fail;
Signature · 6 lines
2.17.1


-- 
Kees Cook
Pixel Security
maintainer Reviewed-by on v1 1/1
#1 lore raw
Quoted 4 lines · Kees Cook

As already done treewide, switch from open-coded multiplication to 2-factor allocation helper.

Signed-off-by Kees Cook <[email protected]>
Reviewed-by Harry Wentland <[email protected]>

Harry

Quoted 21 lines
---
 drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/amd/display/modules/color/color_gamma.c +2 -2
index 98edaefa2b47..ee69c949bfbf 100644
@@ -1723,8 +1723,8 @@ bool  mod_color_calculate_curve(enum dc_transfer_func_predefined trans,
 		kvfree(rgb_regamma);
 	} else if (trans == TRANSFER_FUNCTION_HLG ||
 		trans == TRANSFER_FUNCTION_HLG12) {
-		rgb_regamma = kvzalloc(sizeof(*rgb_regamma) *
-				       (MAX_HW_POINTS + _EXTRA_POINTS),
+		rgb_regamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
+				       sizeof(*rgb_regamma),
 				       GFP_KERNEL);
 		if (!rgb_regamma)
 			goto rgb_regamma_alloc_fail;

drivers/gpu/drm/amd/display/modules/color/color_gamma.c bool mod_color_calculate_degamma_curve(enum dc_transfer_func_predefined trans, · line 1809

Quoted 10 lines
drivers/gpu/drm/amd/display/modules/color/color_gamma.c +2 -2
@@ -1802,8 +1802,8 @@ bool  mod_color_calculate_degamma_curve(enum dc_transfer_func_predefined trans,
 		kvfree(rgb_degamma);
 	} else if (trans == TRANSFER_FUNCTION_HLG ||
 		trans == TRANSFER_FUNCTION_HLG12) {
-		rgb_degamma = kvzalloc(sizeof(*rgb_degamma) *
-				       (MAX_HW_POINTS + _EXTRA_POINTS),
+		rgb_degamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
+				       sizeof(*rgb_degamma),
 				       GFP_KERNEL);
 		if (!rgb_degamma)
 			goto rgb_degamma_alloc_fail;

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

#2 lore raw
In reply to Kees Cook
Quoted 25 lines · Kees Cook

As already done treewide, switch from open-coded multiplication to 2-factor allocation helper.

Signed-off-by Kees Cook <[email protected]>
---
 drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/amd/display/modules/color/color_gamma.c +2 -2
index 98edaefa2b47..ee69c949bfbf 100644
@@ -1723,8 +1723,8 @@ bool  mod_color_calculate_curve(enum dc_transfer_func_predefined trans,
 		kvfree(rgb_regamma);
 	} else if (trans == TRANSFER_FUNCTION_HLG ||
 		trans == TRANSFER_FUNCTION_HLG12) {
-		rgb_regamma = kvzalloc(sizeof(*rgb_regamma) *
-				       (MAX_HW_POINTS + _EXTRA_POINTS),
+		rgb_regamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
+				       sizeof(*rgb_regamma),
 				       GFP_KERNEL);
 		if (!rgb_regamma)
 			goto rgb_regamma_alloc_fail;

drivers/gpu/drm/amd/display/modules/color/color_gamma.c bool mod_color_calculate_degamma_curve(enum dc_transfer_func_predefined trans, · line 1809

Quoted 10 lines
drivers/gpu/drm/amd/display/modules/color/color_gamma.c +2 -2
@@ -1802,8 +1802,8 @@ bool  mod_color_calculate_degamma_curve(enum dc_transfer_func_predefined trans,
 		kvfree(rgb_degamma);
 	} else if (trans == TRANSFER_FUNCTION_HLG ||
 		trans == TRANSFER_FUNCTION_HLG12) {
-		rgb_degamma = kvzalloc(sizeof(*rgb_degamma) *
-				       (MAX_HW_POINTS + _EXTRA_POINTS),
+		rgb_degamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
+				       sizeof(*rgb_degamma),
 				       GFP_KERNEL);
 		if (!rgb_degamma)
 			goto rgb_degamma_alloc_fail;

Since the values are constant, kvcalloc incurs the overflow checking overhead for no gain.

Signature · 6 lines
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#3 lore raw
Quoted 31 lines · Michel Dänzer
Quoted 30 lines · Kees Cook

As already done treewide, switch from open-coded multiplication to 2-factor allocation helper.

Signed-off-by Kees Cook <[email protected]>
---
 drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/amd/display/modules/color/color_gamma.c +3 -4
index 98edaefa2b47..ee69c949bfbf 100644
@@ -1723,8 +1723,8 @@ bool  mod_color_calculate_curve(enum dc_transfer_func_predefined trans,
              kvfree(rgb_regamma);
      } else if (trans == TRANSFER_FUNCTION_HLG ||
              trans == TRANSFER_FUNCTION_HLG12) {
-             rgb_regamma = kvzalloc(sizeof(*rgb_regamma) *
-                                    (MAX_HW_POINTS + _EXTRA_POINTS),
+             rgb_regamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
+                                    sizeof(*rgb_regamma),
                                     GFP_KERNEL);
              if (!rgb_regamma)
                      goto rgb_regamma_alloc_fail;

@@ -1802,8 +1802,8 @@ bool  mod_color_calculate_degamma_curve(enum dc_transfer_func_predefined trans,
              kvfree(rgb_degamma);
      } else if (trans == TRANSFER_FUNCTION_HLG ||
              trans == TRANSFER_FUNCTION_HLG12) {
-             rgb_degamma = kvzalloc(sizeof(*rgb_degamma) *
-                                    (MAX_HW_POINTS + _EXTRA_POINTS),
+             rgb_degamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS,
Quoted 8 lines

drivers/gpu/drm/amd/display/modules/color/color_gamma.c bool mod_color_calculate_degamma_curve(enum dc_transfer_func_predefined trans, · line 1809

Quoted 5 lines
drivers/gpu/drm/amd/display/modules/color/color_gamma.c +1 -0
+                                    sizeof(*rgb_degamma),
                                     GFP_KERNEL);
              if (!rgb_degamma)
                      goto rgb_degamma_alloc_fail;

Since the values are constant, kvcalloc incurs the overflow checking overhead for no gain.

The way the macros are designed, they'll get entirely optimized away in that case:
http://lkml.kernel.org/r/CAGXu5jLw9kMZRkzL_Q7JdyyprntHF6bdVSE_vjyzect3CenDng@mail.gmail.com

Using this patch means if the code ever changes away from constants, it'll still be safe. And static checkers won't yell about the open-coded multiplication here. So it's only up-sides. :)

-Kees

Signature · 2 lines
Kees Cook
Pixel Security