| Message ID | 20201008143450.GA23077@embeddedor (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [next] amd/amdgpu_ctx: Use struct_size() helper and kmalloc() | expand |
Applied. Thanks! Alex On Thu, Oct 8, 2020 at 10:29 AM Gustavo A. R. Silva <gustavoars@kernel.org> wrote: > > Make use of the new struct_size() helper instead of the offsetof() idiom. > Also, use kmalloc() instead of kcalloc(). > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > index c80d8339f58c..5be125f3b92a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > @@ -100,7 +100,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip, > enum drm_sched_priority priority; > int r; > > - entity = kcalloc(1, offsetof(typeof(*entity), fences[amdgpu_sched_jobs]), > + entity = kmalloc(struct_size(entity, fences, amdgpu_sched_jobs), > GFP_KERNEL); > if (!entity) > return -ENOMEM; > -- > 2.27.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Am 08.10.20 um 16:34 schrieb Gustavo A. R. Silva: > Make use of the new struct_size() helper instead of the offsetof() idiom. > Also, use kmalloc() instead of kcalloc(). > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > index c80d8339f58c..5be125f3b92a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > @@ -100,7 +100,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip, > enum drm_sched_priority priority; > int r; > > - entity = kcalloc(1, offsetof(typeof(*entity), fences[amdgpu_sched_jobs]), > + entity = kmalloc(struct_size(entity, fences, amdgpu_sched_jobs), NAK. You could use kzalloc() here, but kmalloc won't zero initialize the memory which could result in unforeseen consequences. Christian. > GFP_KERNEL); > if (!entity) > return -ENOMEM;
On Fri, Oct 09, 2020 at 09:08:51AM +0200, Christian König wrote: > Am 08.10.20 um 16:34 schrieb Gustavo A. R. Silva: > > Make use of the new struct_size() helper instead of the offsetof() idiom. > > Also, use kmalloc() instead of kcalloc(). > > > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > > index c80d8339f58c..5be125f3b92a 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > > @@ -100,7 +100,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip, > > enum drm_sched_priority priority; > > int r; > > - entity = kcalloc(1, offsetof(typeof(*entity), fences[amdgpu_sched_jobs]), > > + entity = kmalloc(struct_size(entity, fences, amdgpu_sched_jobs), > > NAK. You could use kzalloc() here, but kmalloc won't zero initialize the > memory which could result in unforeseen consequences. Oh I see.. I certainly didn't take that into account. I'll fix that up and respin. Thanks -- Gustavo
Am 09.10.20 um 15:54 schrieb Gustavo A. R. Silva: > On Fri, Oct 09, 2020 at 09:08:51AM +0200, Christian König wrote: >> Am 08.10.20 um 16:34 schrieb Gustavo A. R. Silva: >>> Make use of the new struct_size() helper instead of the offsetof() idiom. >>> Also, use kmalloc() instead of kcalloc(). >>> >>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c >>> index c80d8339f58c..5be125f3b92a 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c >>> @@ -100,7 +100,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip, >>> enum drm_sched_priority priority; >>> int r; >>> - entity = kcalloc(1, offsetof(typeof(*entity), fences[amdgpu_sched_jobs]), >>> + entity = kmalloc(struct_size(entity, fences, amdgpu_sched_jobs), >> NAK. You could use kzalloc() here, but kmalloc won't zero initialize the >> memory which could result in unforeseen consequences. > Oh I see.. I certainly didn't take that into account. > > I'll fix that up and respin. Shit happens, we already have a fix for this. Alex merged it and it immediately broke our testing systems. So one of our engineers came up with a fix which should already have been applied. Christian. > > Thanks > -- > Gustavo
On Fri, Oct 09, 2020 at 04:29:55PM +0200, Christian König wrote: > > > > - entity = kcalloc(1, offsetof(typeof(*entity), fences[amdgpu_sched_jobs]), > > > > + entity = kmalloc(struct_size(entity, fences, amdgpu_sched_jobs), > > > NAK. You could use kzalloc() here, but kmalloc won't zero initialize the > > > memory which could result in unforeseen consequences. > > Oh I see.. I certainly didn't take that into account. > > > > I'll fix that up and respin. > > Shit happens, we already have a fix for this. Alex merged it and it > immediately broke our testing systems. :/ > So one of our engineers came up with a fix which should already have been > applied. Great. Good to know it's already fixed! :) Thanks -- Gustavo
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c index c80d8339f58c..5be125f3b92a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c @@ -100,7 +100,7 @@ static int amdgpu_ctx_init_entity(struct amdgpu_ctx *ctx, u32 hw_ip, enum drm_sched_priority priority; int r; - entity = kcalloc(1, offsetof(typeof(*entity), fences[amdgpu_sched_jobs]), + entity = kmalloc(struct_size(entity, fences, amdgpu_sched_jobs), GFP_KERNEL); if (!entity) return -ENOMEM;
Make use of the new struct_size() helper instead of the offsetof() idiom. Also, use kmalloc() instead of kcalloc(). Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)