[PATCH] gpu: drm: use struct_size() in kmalloc()

[PATCH] gpu: drm: use struct_size() in kmalloc()

[xiaolinkui]

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

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Pan, Xinhui]

[-- Attachment #1.1: Type: text/plain, Size: 1484 bytes --]

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.
________________________________
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




[-- Attachment #1.2: Type: text/html, Size: 2930 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Daniel Vetter]

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

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Pan, Xinhui]

[-- Attachment #1.1: Type: text/plain, Size: 2533 bytes --]

Daniel, your idea is obviously and totally wrong. There can NOT be more than one zero-size array in a struct.

Nack for them all.
________________________________
From: Daniel Vetter <daniel.vetter@ffwll.ch> on behalf of Daniel Vetter <daniel@ffwll.ch>
Sent: Tuesday, May 21, 2019 12:28:07 AM
To: Pan, Xinhui
Cc: 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
Subject: 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

[-- Attachment #1.2: Type: text/html, Size: 4409 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

回复: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Pan, Xinhui]

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. 


发件人: 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

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Alex Deucher]

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.

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

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[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/

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.

Except in really hot paths I personally think that that kind of
trickery isn't worth it.

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



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Christian König]

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

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Gustavo A. R. Silva]

On 5/21/19 3:59 AM, Christian König wrote:

> 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.
> 

A flexible array member is the preferred[1] mechanism:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190520&id=76497732932f15e7323dc805e8ea8dc11bb587cf

Thanks
--
Gustavo

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Alex Deucher]

On Fri, May 17, 2019 at 8:43 AM xiaolinkui <xiaolinkui@kylinos.cn> wrote:
>
> 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(-)

This patch results in the following build error:
  DESCEND  objtool
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.o
In file included from ./include/linux/kernel.h:15,
                 from ./include/linux/list.h:9,
                 from ./include/linux/wait.h:7,
                 from ./include/linux/wait_bit.h:8,
                 from ./include/linux/fs.h:6,
                 from ./include/linux/debugfs.h:15,
                 from drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:24:
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c: In function ‘amdgpu_ras_init’:
./include/linux/build_bug.h:16:45: error: negative width in bit-field
‘<anonymous>’
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
                                             ^
./include/linux/compiler.h:349:28: note: in expansion of macro
‘BUILD_BUG_ON_ZERO’
 #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
                            ^~~~~~~~~~~~~~~~~
./include/linux/overflow.h:306:30: note: in expansion of macro ‘__must_be_array’
       sizeof(*(p)->member) + __must_be_array((p)->member),\
                              ^~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1556:16: note: in expansion of
macro ‘struct_size’
  con = kmalloc(struct_size(con, objs, AMDGPU_RAS_BLOCK_COUNT),
                ^~~~~~~~~~~
make[4]: *** [scripts/Makefile.build:276:
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:492: drivers/gpu/drm/amd/amdgpu] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:492: drivers/gpu/drm] Error 2
make[1]: *** [scripts/Makefile.build:492: drivers/gpu] Error 2
make: *** [Makefile:1042: drivers] Error 2

Alex


>
> 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
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Gustavo A. R. Silva]

On 5/20/19 12:41 PM, Alex Deucher wrote:
> On Fri, May 17, 2019 at 8:43 AM xiaolinkui <xiaolinkui@kylinos.cn> wrote:
>>
>> Use struct_size() helper to keep code simple.
>>

Again, this is not the reason why this helper was created.

>> Signed-off-by: xiaolinkui <xiaolinkui@kylinos.cn>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> This patch results in the following build error:
>   DESCEND  objtool
>   CALL    scripts/checksyscalls.sh
>   CHK     include/generated/compile.h
>   CC [M]  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.o
> In file included from ./include/linux/kernel.h:15,
>                  from ./include/linux/list.h:9,
>                  from ./include/linux/wait.h:7,
>                  from ./include/linux/wait_bit.h:8,
>                  from ./include/linux/fs.h:6,
>                  from ./include/linux/debugfs.h:15,
>                  from drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:24:
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c: In function ‘amdgpu_ras_init’:
> ./include/linux/build_bug.h:16:45: error: negative width in bit-field
> ‘<anonymous>’
>  #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
>                                              ^
> ./include/linux/compiler.h:349:28: note: in expansion of macro
> ‘BUILD_BUG_ON_ZERO’
>  #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
>                             ^~~~~~~~~~~~~~~~~
> ./include/linux/overflow.h:306:30: note: in expansion of macro ‘__must_be_array’
>        sizeof(*(p)->member) + __must_be_array((p)->member),\
>                               ^~~~~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1556:16: note: in expansion of
> macro ‘struct_size’
>   con = kmalloc(struct_size(con, objs, AMDGPU_RAS_BLOCK_COUNT),
>                 ^~~~~~~~~~~
> make[4]: *** [scripts/Makefile.build:276:
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> make[3]: *** [scripts/Makefile.build:492: drivers/gpu/drm/amd/amdgpu] Error 2
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [scripts/Makefile.build:492: drivers/gpu/drm] Error 2
> make[1]: *** [scripts/Makefile.build:492: drivers/gpu] Error 2
> make: *** [Makefile:1042: drivers] Error 2
> 
> Alex
> 

You continue[1][2] sending these sorts of patches without really understanding what
you are doing. And you don't even compile them. Why?

--
Gustavo

[1] https://lore.kernel.org/lkml/d83390a9-33be-3d76-3e23-b97f0a05b72f@kernel.dk/
[2] https://lore.kernel.org/lkml/b4d33107-75d5-fa18-536e-6d21c96e4972@kernel.dk/

> 
>>
>> 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
>>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx