From: "Christian König" <ckoenig.leichtzumerken@gmail.com> To: Daniel Vetter <daniel@ffwll.ch>, Alex Deucher <alexdeucher@gmail.com> Cc: "Zhou, David(ChunMing)" <David1.Zhou@amd.com>, "airlied@linux.ie" <airlied@linux.ie>, "Pan, Xinhui" <Xinhui.Pan@amd.com>, xiaolinkui <xiaolinkui@kylinos.cn>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, "Deucher, Alexander" <Alexander.Deucher@amd.com>, "Quan, Evan" <Evan.Quan@amd.com>, "Koenig, Christian" <Christian.Koenig@amd.com> Subject: Re: [PATCH] gpu: drm: use struct_size() in kmalloc() Date: Tue, 21 May 2019 10:59:06 +0200 [thread overview] Message-ID: <36de7e05-9055-6d8b-fb2c-fa5a4e94274b@gmail.com> (raw) In-Reply-To: <CAKMK7uHS837L9Ze_K5q-AsFgOtAMD+n_i_Y404BX-_CwJeP08Q@mail.gmail.com> Am 21.05.19 um 09:23 schrieb Daniel Vetter: > On Tue, May 21, 2019 at 4:33 AM Alex Deucher <alexdeucher@gmail.com> wrote: >> On Mon, May 20, 2019 at 7:19 PM Pan, Xinhui <Xinhui.Pan@amd.com> wrote: >>> Daniel, what you are talking about is totally wrong. >>> 1) AFAIK, only one zero-size array can be in the end of a struct. >>> 2) two struct_size will add up struct itself twice. the sum is wrong then. >>> >>> No offense. I can't help feeling lucky that you are in intel. >> Xinhui, >> >> Please keep things civil. There is no need for comments like this. > Yeah, this was over the line, thanks Alex for already taking care of > this. Please note that fd.o mailing lists operate under a CoC: > > https://www.freedesktop.org/wiki/CodeOfConduct/ Seconded. I also enjoy the humiliation of other in email, but it doesn't helps us getting code written and problems solved in a professional environment. > Wrt the technical comment: I know that you can only do one variable > sized array, and it must be at the end. But you can put multiple > structures all within the same allocation. Which is what I thought you > wanted to do. And my sketch would allow you to do that even if you > have multiple variable length structures you want to allocate. There's > plenty examples of this (but open-coded ones) in the kernel. BTW: Is there actually good documentation how to correctly do the variable length array at end of structure thing in the kernel? I do know that I've seen a lot of different variants like array[] array[0] or array[1] and I have also seen a bunch of gcc versions failing to generate correct code for some of them. So we should probably nail down how to do things correctly. > Except in really hot paths I personally think that that kind of > trickery isn't worth it. Well for kmalloc() it's not that much overhead, but with vmalloc that is a completely different picture. Christian. > > Cheers, Daniel > >> Alex >> >>> >>> 发件人: Daniel Vetter <daniel.vetter@ffwll.ch> 代表 Daniel Vetter <daniel@ffwll.ch> >>> 发送时间: 2019年5月21日 0:28 >>> 收件人: Pan, Xinhui >>> 抄送: Deucher, Alexander; Koenig, Christian; Zhou, David(ChunMing); airlied@linux.ie; daniel@ffwll.ch; Quan, Evan; xiaolinkui; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org >>> 主题: Re: [PATCH] gpu: drm: use struct_size() in kmalloc() >>> >>> [CAUTION: External Email] >>> >>> On Fri, May 17, 2019 at 04:44:30PM +0000, Pan, Xinhui wrote: >>>> I am going to put more members which are also array after this struct, >>>> not only obj[]. Looks like this struct_size did not help on multiple >>>> array case. Thanks anyway. ________________________________ >>> You can then add them up, e.g. kmalloc(struct_size()+struct_size(), >>> GFP_KERNEL), so this patch here still looks like a good idea. >>> >>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >>> >>> Cheers, Daniel >>> >>>> From: xiaolinkui <xiaolinkui@kylinos.cn> >>>> Sent: Friday, May 17, 2019 4:46:00 PM >>>> To: Deucher, Alexander; Koenig, Christian; Zhou, David(ChunMing); airlied@linux.ie; daniel@ffwll.ch; Pan, Xinhui; Quan, Evan >>>> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; xiaolinkui@kylinos.cn >>>> Subject: [PATCH] gpu: drm: use struct_size() in kmalloc() >>>> >>>> [CAUTION: External Email] >>>> >>>> Use struct_size() helper to keep code simple. >>>> >>>> Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c >>>> index 22bd21e..4717a64 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c >>>> @@ -1375,8 +1375,7 @@ int amdgpu_ras_init(struct amdgpu_device *adev) >>>> if (con) >>>> return 0; >>>> >>>> - con = kmalloc(sizeof(struct amdgpu_ras) + >>>> - sizeof(struct ras_manager) * AMDGPU_RAS_BLOCK_COUNT, >>>> + con = kmalloc(struct_size(con, objs, AMDGPU_RAS_BLOCK_COUNT), >>>> GFP_KERNEL|__GFP_ZERO); >>>> if (!con) >>>> return -ENOMEM; >>>> -- >>>> 2.7.4 >>>> >>>> >>>> >>> -- >>> Daniel Vetter >>> Software Engineer, Intel Corporation >>> http://blog.ffwll.ch >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > >
WARNING: multiple messages have this Message-ID (diff)
From: "Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> To: Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org>, Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Cc: "Zhou, David(ChunMing)" <David1.Zhou-5C7GfCeVMHo@public.gmane.org>, "airlied-cv59FeDIM0c@public.gmane.org" <airlied-cv59FeDIM0c@public.gmane.org>, "Pan, Xinhui" <Xinhui.Pan-5C7GfCeVMHo@public.gmane.org>, xiaolinkui <xiaolinkui-UOlijcLmZ/InDS1+zs4M5A@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>, "dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" <dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>, "Deucher, Alexander" <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>, "Quan, Evan" <Evan.Quan-5C7GfCeVMHo@public.gmane.org>, "Koenig, Christian" <Christian.Koenig-5C7GfCeVMHo@public.gmane.org> Subject: Re: [PATCH] gpu: drm: use struct_size() in kmalloc() Date: Tue, 21 May 2019 10:59:06 +0200 [thread overview] Message-ID: <36de7e05-9055-6d8b-fb2c-fa5a4e94274b@gmail.com> (raw) In-Reply-To: <CAKMK7uHS837L9Ze_K5q-AsFgOtAMD+n_i_Y404BX-_CwJeP08Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> Am 21.05.19 um 09:23 schrieb Daniel Vetter: > On Tue, May 21, 2019 at 4:33 AM Alex Deucher <alexdeucher@gmail.com> wrote: >> On Mon, May 20, 2019 at 7:19 PM Pan, Xinhui <Xinhui.Pan@amd.com> wrote: >>> Daniel, what you are talking about is totally wrong. >>> 1) AFAIK, only one zero-size array can be in the end of a struct. >>> 2) two struct_size will add up struct itself twice. the sum is wrong then. >>> >>> No offense. I can't help feeling lucky that you are in intel. >> Xinhui, >> >> Please keep things civil. There is no need for comments like this. > Yeah, this was over the line, thanks Alex for already taking care of > this. Please note that fd.o mailing lists operate under a CoC: > > https://www.freedesktop.org/wiki/CodeOfConduct/ Seconded. I also enjoy the humiliation of other in email, but it doesn't helps us getting code written and problems solved in a professional environment. > Wrt the technical comment: I know that you can only do one variable > sized array, and it must be at the end. But you can put multiple > structures all within the same allocation. Which is what I thought you > wanted to do. And my sketch would allow you to do that even if you > have multiple variable length structures you want to allocate. There's > plenty examples of this (but open-coded ones) in the kernel. BTW: Is there actually good documentation how to correctly do the variable length array at end of structure thing in the kernel? I do know that I've seen a lot of different variants like array[] array[0] or array[1] and I have also seen a bunch of gcc versions failing to generate correct code for some of them. So we should probably nail down how to do things correctly. > Except in really hot paths I personally think that that kind of > trickery isn't worth it. Well for kmalloc() it's not that much overhead, but with vmalloc that is a completely different picture. Christian. > > Cheers, Daniel > >> Alex >> >>> >>> 发件人: Daniel Vetter <daniel.vetter@ffwll.ch> 代表 Daniel Vetter <daniel@ffwll.ch> >>> 发送时间: 2019年5月21日 0:28 >>> 收件人: Pan, Xinhui >>> 抄送: Deucher, Alexander; Koenig, Christian; Zhou, David(ChunMing); airlied@linux.ie; daniel@ffwll.ch; Quan, Evan; xiaolinkui; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org >>> 主题: Re: [PATCH] gpu: drm: use struct_size() in kmalloc() >>> >>> [CAUTION: External Email] >>> >>> On Fri, May 17, 2019 at 04:44:30PM +0000, Pan, Xinhui wrote: >>>> I am going to put more members which are also array after this struct, >>>> not only obj[]. Looks like this struct_size did not help on multiple >>>> array case. Thanks anyway. ________________________________ >>> You can then add them up, e.g. kmalloc(struct_size()+struct_size(), >>> GFP_KERNEL), so this patch here still looks like a good idea. >>> >>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> >>> >>> Cheers, Daniel >>> >>>> From: xiaolinkui <xiaolinkui@kylinos.cn> >>>> Sent: Friday, May 17, 2019 4:46:00 PM >>>> To: Deucher, Alexander; Koenig, Christian; Zhou, David(ChunMing); airlied@linux.ie; daniel@ffwll.ch; Pan, Xinhui; Quan, Evan >>>> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; xiaolinkui@kylinos.cn >>>> Subject: [PATCH] gpu: drm: use struct_size() in kmalloc() >>>> >>>> [CAUTION: External Email] >>>> >>>> Use struct_size() helper to keep code simple. >>>> >>>> Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c >>>> index 22bd21e..4717a64 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c >>>> @@ -1375,8 +1375,7 @@ int amdgpu_ras_init(struct amdgpu_device *adev) >>>> if (con) >>>> return 0; >>>> >>>> - con = kmalloc(sizeof(struct amdgpu_ras) + >>>> - sizeof(struct ras_manager) * AMDGPU_RAS_BLOCK_COUNT, >>>> + con = kmalloc(struct_size(con, objs, AMDGPU_RAS_BLOCK_COUNT), >>>> GFP_KERNEL|__GFP_ZERO); >>>> if (!con) >>>> return -ENOMEM; >>>> -- >>>> 2.7.4 >>>> >>>> >>>> >>> -- >>> Daniel Vetter >>> Software Engineer, Intel Corporation >>> http://blog.ffwll.ch >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2019-05-21 8:59 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-17 8:46 [PATCH] gpu: drm: use struct_size() in kmalloc() xiaolinkui 2019-05-17 16:44 ` Pan, Xinhui 2019-05-20 16:28 ` Daniel Vetter 2019-05-20 16:28 ` Daniel Vetter 2019-05-20 22:54 ` Pan, Xinhui 2019-05-20 23:19 ` 回复: " Pan, Xinhui 2019-05-20 23:19 ` Pan, Xinhui 2019-05-21 2:32 ` Alex Deucher 2019-05-21 7:23 ` Daniel Vetter 2019-05-21 8:59 ` Christian König [this message] 2019-05-21 8:59 ` Christian König 2019-05-21 12:57 ` Gustavo A. R. Silva 2019-05-20 17:41 ` Alex Deucher 2019-05-20 17:41 ` Alex Deucher 2019-05-20 18:21 ` Gustavo A. R. Silva 2019-05-20 18:21 ` Gustavo A. R. Silva
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=36de7e05-9055-6d8b-fb2c-fa5a4e94274b@gmail.com \ --to=ckoenig.leichtzumerken@gmail.com \ --cc=Alexander.Deucher@amd.com \ --cc=Christian.Koenig@amd.com \ --cc=David1.Zhou@amd.com \ --cc=Evan.Quan@amd.com \ --cc=Xinhui.Pan@amd.com \ --cc=airlied@linux.ie \ --cc=alexdeucher@gmail.com \ --cc=amd-gfx@lists.freedesktop.org \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ --cc=xiaolinkui@kylinos.cn \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.