LGTM and Thanks for the patch! Reviewed-by: Zhi Wang <zhi.a.wang@xxxxxxxxx> -----Original Message----- From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] Sent: Monday, October 16, 2017 12:35 PM To: Jérémy Lefaure <jeremy.lefaure@xxxxxxxxxxxx>; Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx> Cc: Jérémy Lefaure <jeremy.lefaure@xxxxxxxxxxxx>; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx Subject: Re: [PATCH] drm/i915/gvt: use ARRAY_SIZE [dropped a number of lists and maintainers, please use common sense] On Sun, 15 Oct 2017, Jérémy Lefaure <jeremy.lefaure@xxxxxxxxxxxx> wrote: > Using the ARRAY_SIZE macro improves the readability of the code. Also, > it's useless to use a variable to store this constant calculated at > compile time. I'll leave it up to Zhenyu and Zhi, but IMHO the variable is not useless. It improves the readability of the code by giving a name to the information, and the compiler will throw it away. Either way, Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> (Up next, someone sends another patch to change the kzalloc to kcalloc... *sigh*) > > Found with Coccinelle with the following semantic patch: > @r depends on (org || report)@ > type T; > T[] E; > position p; > @@ > ( > (sizeof(E)@p /sizeof(*E)) > | > (sizeof(E)@p /sizeof(E[...])) > | > (sizeof(E)@p /sizeof(T)) > ) > > Signed-off-by: Jérémy Lefaure <jeremy.lefaure@xxxxxxxxxxxx> > --- > This patch was part of a bigger patch [1]. > > [1]: https://patchwork.kernel.org/patch/9979843/ > > drivers/gpu/drm/i915/gvt/vgpu.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c > b/drivers/gpu/drm/i915/gvt/vgpu.c index 02c61a1ad56a..b32c1c889ea8 > 100644 > --- a/drivers/gpu/drm/i915/gvt/vgpu.c > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c > @@ -31,6 +31,7 @@ > * > */ > > +#include <linux/kernel.h> > #include "i915_drv.h" > #include "gvt.h" > #include "i915_pvinfo.h" > @@ -98,7 +99,6 @@ static struct { > */ > int intel_gvt_init_vgpu_types(struct intel_gvt *gvt) { > - unsigned int num_types; > unsigned int i, low_avail, high_avail; > unsigned int min_low; > > @@ -116,15 +116,14 @@ int intel_gvt_init_vgpu_types(struct intel_gvt *gvt) > */ > low_avail = gvt_aperture_sz(gvt) - HOST_LOW_GM_SIZE; > high_avail = gvt_hidden_sz(gvt) - HOST_HIGH_GM_SIZE; > - num_types = sizeof(vgpu_types) / sizeof(vgpu_types[0]); > > - gvt->types = kzalloc(num_types * sizeof(struct intel_vgpu_type), > - GFP_KERNEL); > + gvt->types = kzalloc(ARRAY_SIZE(vgpu_types) * > + sizeof(struct intel_vgpu_type), GFP_KERNEL); > if (!gvt->types) > return -ENOMEM; > > min_low = MB_TO_BYTES(32); > - for (i = 0; i < num_types; ++i) { > + for (i = 0; i < ARRAY_SIZE(vgpu_types); ++i) { > if (low_avail / vgpu_types[i].low_mm == 0) > break; -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
