* [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' @ 2019-06-22 3:03 Mao Wenan 2019-06-22 6:02 ` Julia Lawall 2019-06-22 10:43 ` Dan Carpenter 0 siblings, 2 replies; 22+ messages in thread From: Mao Wenan @ 2019-06-22 3:03 UTC (permalink / raw) To: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou Cc: kernel-janitors, amd-gfx, linux-kernel, Mao Wenan Fixes gcc '-Wunused-but-set-variable' warning: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] int ret = 0; ^ It is never used since introduction in 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") Signed-off-by: Mao Wenan <maowenan@huawei.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 0e6dba9..0bf4dd9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev, /* init amdgpu_pmu */ int amdgpu_pmu_init(struct amdgpu_device *adev) { - int ret = 0; - switch (adev->asic_type) { case CHIP_VEGA20: /* init df */ - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, + init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS); -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' 2019-06-22 3:03 [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' Mao Wenan @ 2019-06-22 6:02 ` Julia Lawall 2019-06-22 7:20 ` maowenan 2019-06-22 10:43 ` Dan Carpenter 1 sibling, 1 reply; 22+ messages in thread From: Julia Lawall @ 2019-06-22 6:02 UTC (permalink / raw) To: Mao Wenan Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, kernel-janitors, amd-gfx, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1410 bytes --] On Sat, 22 Jun 2019, Mao Wenan wrote: > Fixes gcc '-Wunused-but-set-variable' warning: > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] > int ret = 0; > ^ > > It is never used since introduction in 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") > > Signed-off-by: Mao Wenan <maowenan@huawei.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > index 0e6dba9..0bf4dd9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev, > /* init amdgpu_pmu */ > int amdgpu_pmu_init(struct amdgpu_device *adev) > { > - int ret = 0; > - > switch (adev->asic_type) { > case CHIP_VEGA20: > /* init df */ > - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > + init_pmu_by_type(adev, df_v3_6_attr_groups, > "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > DF_V3_6_MAX_COUNTERS); Maybe it would be better to use ret? If knowing whether the call has failed is really useless, then maybe the return type should be void? julia > > -- > 2.7.4 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' 2019-06-22 6:02 ` Julia Lawall @ 2019-06-22 7:20 ` maowenan 0 siblings, 0 replies; 22+ messages in thread From: maowenan @ 2019-06-22 7:20 UTC (permalink / raw) To: Julia Lawall Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, kernel-janitors, amd-gfx, linux-kernel On 2019/6/22 14:02, Julia Lawall wrote: > > > On Sat, 22 Jun 2019, Mao Wenan wrote: > >> Fixes gcc '-Wunused-but-set-variable' warning: >> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] >> int ret = 0; >> ^ >> >> It is never used since introduction in 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") >> >> Signed-off-by: Mao Wenan <maowenan@huawei.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> index 0e6dba9..0bf4dd9 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev, >> /* init amdgpu_pmu */ >> int amdgpu_pmu_init(struct amdgpu_device *adev) >> { >> - int ret = 0; >> - >> switch (adev->asic_type) { >> case CHIP_VEGA20: >> /* init df */ >> - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, >> + init_pmu_by_type(adev, df_v3_6_attr_groups, >> "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> DF_V3_6_MAX_COUNTERS); > > Maybe it would be better to use ret? > > If knowing whether the call has failed is really useless, then maybe the > return type should be void? right. amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, which will use the return value. amdgpu_device_init() r = amdgpu_pmu_init(adev); thanks, I will send v2. > > julia > > >> >> -- >> 2.7.4 >> >> > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' 2019-06-22 3:03 [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' Mao Wenan 2019-06-22 6:02 ` Julia Lawall @ 2019-06-22 10:43 ` Dan Carpenter 2019-06-22 13:05 ` [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init Mao Wenan 2019-06-23 6:00 ` [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' Dan Carpenter 1 sibling, 2 replies; 22+ messages in thread From: Dan Carpenter @ 2019-06-22 10:43 UTC (permalink / raw) To: Mao Wenan Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, kernel-janitors, amd-gfx, linux-kernel On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote: > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > index 0e6dba9..0bf4dd9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev, > /* init amdgpu_pmu */ > int amdgpu_pmu_init(struct amdgpu_device *adev) > { > - int ret = 0; > - > switch (adev->asic_type) { > case CHIP_VEGA20: > /* init df */ > - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > + init_pmu_by_type(adev, df_v3_6_attr_groups, > "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > DF_V3_6_MAX_COUNTERS); You're resending this for other reasons, but don't forget to update the indenting on the arguments so they still line up with the '('. regards, dan carpenter ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init 2019-06-22 10:43 ` Dan Carpenter @ 2019-06-22 13:05 ` Mao Wenan 2019-06-22 13:06 ` Julia Lawall 2019-06-22 18:13 ` Joe Perches 2019-06-23 6:00 ` [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' Dan Carpenter 1 sibling, 2 replies; 22+ messages in thread From: Mao Wenan @ 2019-06-22 13:05 UTC (permalink / raw) To: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel, Mao Wenan There is one warning: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] int ret = 0; ^ amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, which will use the return value. So it returns 'ret' for caller. amdgpu_device_init() r = amdgpu_pmu_init(adev); Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") Signed-off-by: Mao Wenan <maowenan@huawei.com> --- v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in amdgpu_pmu_init(). drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 0e6dba9..145e720 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) case CHIP_VEGA20: /* init df */ ret = init_pmu_by_type(adev, df_v3_6_attr_groups, - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, - DF_V3_6_MAX_COUNTERS); + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, + DF_V3_6_MAX_COUNTERS); /* other pmu types go here*/ break; @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) return 0; } - return 0; + return ret; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init 2019-06-22 13:05 ` [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init Mao Wenan @ 2019-06-22 13:06 ` Julia Lawall 2019-06-22 13:56 ` maowenan 2019-06-22 18:13 ` Joe Perches 1 sibling, 1 reply; 22+ messages in thread From: Julia Lawall @ 2019-06-22 13:06 UTC (permalink / raw) To: Mao Wenan Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, kernel-janitors, amd-gfx, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1830 bytes --] On Sat, 22 Jun 2019, Mao Wenan wrote: > There is one warning: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] > int ret = 0; > ^ > amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, > which will use the return value. So it returns 'ret' for caller. > amdgpu_device_init() > r = amdgpu_pmu_init(adev); > > Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") > > Signed-off-by: Mao Wenan <maowenan@huawei.com> > --- > v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in > amdgpu_pmu_init(). > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > index 0e6dba9..145e720 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > case CHIP_VEGA20: > /* init df */ > ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > - DF_V3_6_MAX_COUNTERS); > + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > + DF_V3_6_MAX_COUNTERS); > > /* other pmu types go here*/ I don't know what is the impact of the other pmu types that are planned for the future. Perhaps it would be better to abort the function immediately in the case of a failure. julia > break; > @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > return 0; > } > > - return 0; > + return ret; > } > > > -- > 2.7.4 > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init 2019-06-22 13:06 ` Julia Lawall @ 2019-06-22 13:56 ` maowenan 2019-06-22 14:00 ` Julia Lawall 0 siblings, 1 reply; 22+ messages in thread From: maowenan @ 2019-06-22 13:56 UTC (permalink / raw) To: Julia Lawall Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, kernel-janitors, amd-gfx, linux-kernel On 2019/6/22 21:06, Julia Lawall wrote: > > > On Sat, 22 Jun 2019, Mao Wenan wrote: > >> There is one warning: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] >> int ret = 0; >> ^ >> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, >> which will use the return value. So it returns 'ret' for caller. >> amdgpu_device_init() >> r = amdgpu_pmu_init(adev); >> >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") >> >> Signed-off-by: Mao Wenan <maowenan@huawei.com> >> --- >> v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in >> amdgpu_pmu_init(). >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> index 0e6dba9..145e720 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >> case CHIP_VEGA20: >> /* init df */ >> ret = init_pmu_by_type(adev, df_v3_6_attr_groups, >> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> - DF_V3_6_MAX_COUNTERS); >> + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> + DF_V3_6_MAX_COUNTERS); >> >> /* other pmu types go here*/ > > I don't know what is the impact of the other pmu types that are planned > for the future. Perhaps it would be better to abort the function > immediately in the case of a failure. I guess it would be better to use new function or new switch case clause to process different PMU types in future. > > julia > >> break; >> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >> return 0; >> } >> >> - return 0; >> + return ret; >> } >> >> >> -- >> 2.7.4 >> >> > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init 2019-06-22 13:56 ` maowenan @ 2019-06-22 14:00 ` Julia Lawall 2019-06-24 3:16 ` maowenan 0 siblings, 1 reply; 22+ messages in thread From: Julia Lawall @ 2019-06-22 14:00 UTC (permalink / raw) To: maowenan Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, kernel-janitors, amd-gfx, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2459 bytes --] On Sat, 22 Jun 2019, maowenan wrote: > > > On 2019/6/22 21:06, Julia Lawall wrote: > > > > > > On Sat, 22 Jun 2019, Mao Wenan wrote: > > > >> There is one warning: > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] > >> int ret = 0; > >> ^ > >> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, > >> which will use the return value. So it returns 'ret' for caller. > >> amdgpu_device_init() > >> r = amdgpu_pmu_init(adev); > >> > >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") > >> > >> Signed-off-by: Mao Wenan <maowenan@huawei.com> > >> --- > >> v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in > >> amdgpu_pmu_init(). > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> index 0e6dba9..145e720 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > >> case CHIP_VEGA20: > >> /* init df */ > >> ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > >> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > >> - DF_V3_6_MAX_COUNTERS); > >> + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > >> + DF_V3_6_MAX_COUNTERS); > >> > >> /* other pmu types go here*/ > > > > I don't know what is the impact of the other pmu types that are planned > > for the future. Perhaps it would be better to abort the function > > immediately in the case of a failure. > > I guess it would be better to use new function or new switch case clause to process different PMU types > in future. I don't know. But normally when an error may occur it is checked for immediately, rather than just letting it go until the end of the function. But maybe the developer know what is planned for the future for this function. julia > > > > > julia > > > >> break; > >> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > >> return 0; > >> } > >> > >> - return 0; > >> + return ret; > >> } > >> > >> > >> -- > >> 2.7.4 > >> > >> > > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init 2019-06-22 14:00 ` Julia Lawall @ 2019-06-24 3:16 ` maowenan 0 siblings, 0 replies; 22+ messages in thread From: maowenan @ 2019-06-24 3:16 UTC (permalink / raw) To: Julia Lawall Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, kernel-janitors, amd-gfx, linux-kernel On 2019/6/22 22:00, Julia Lawall wrote: > > > On Sat, 22 Jun 2019, maowenan wrote: > >> >> >> On 2019/6/22 21:06, Julia Lawall wrote: >>> >>> >>> On Sat, 22 Jun 2019, Mao Wenan wrote: >>> >>>> There is one warning: >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] >>>> int ret = 0; >>>> ^ >>>> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, >>>> which will use the return value. So it returns 'ret' for caller. >>>> amdgpu_device_init() >>>> r = amdgpu_pmu_init(adev); >>>> >>>> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") >>>> >>>> Signed-off-by: Mao Wenan <maowenan@huawei.com> >>>> --- >>>> v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in >>>> amdgpu_pmu_init(). >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >>>> index 0e6dba9..145e720 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >>>> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >>>> case CHIP_VEGA20: >>>> /* init df */ >>>> ret = init_pmu_by_type(adev, df_v3_6_attr_groups, >>>> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >>>> - DF_V3_6_MAX_COUNTERS); >>>> + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >>>> + DF_V3_6_MAX_COUNTERS); >>>> >>>> /* other pmu types go here*/ >>> >>> I don't know what is the impact of the other pmu types that are planned >>> for the future. Perhaps it would be better to abort the function >>> immediately in the case of a failure. >> OK, v3 will be sent. >> I guess it would be better to use new function or new switch case clause to process different PMU types >> in future. > > I don't know. But normally when an error may occur it is checked for > immediately, rather than just letting it go until the end of the function. > But maybe the developer know what is planned for the future for this > function. > > julia > >> >>> >>> julia >>> >>>> break; >>>> @@ -261,7 +261,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >>>> return 0; >>>> } >>>> >>>> - return 0; >>>> + return ret; >>>> } >>>> >>>> >>>> -- >>>> 2.7.4 >>>> >>>> >>> >> >> > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init 2019-06-22 13:05 ` [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init Mao Wenan 2019-06-22 13:06 ` Julia Lawall @ 2019-06-22 18:13 ` Joe Perches 2019-06-24 3:41 ` maowenan 1 sibling, 1 reply; 22+ messages in thread From: Joe Perches @ 2019-06-22 18:13 UTC (permalink / raw) To: Mao Wenan, airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel, Jonathan Kim On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote: > There is one warning: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] > int ret = 0; [] > v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in > amdgpu_pmu_init(). [] > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c [] > @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > case CHIP_VEGA20: > /* init df */ > ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > - DF_V3_6_MAX_COUNTERS); > + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > + DF_V3_6_MAX_COUNTERS); trivia: The indentation change seems superfluous and appears to make the code harder to read. You could also cc Jonathan Kim who wrote all of this. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init 2019-06-22 18:13 ` Joe Perches @ 2019-06-24 3:41 ` maowenan 2019-06-24 3:46 ` Joe Perches 0 siblings, 1 reply; 22+ messages in thread From: maowenan @ 2019-06-24 3:41 UTC (permalink / raw) To: Joe Perches, airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel, Jonathan Kim On 2019/6/23 2:13, Joe Perches wrote: > On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote: >> There is one warning: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] >> int ret = 0; > [] >> v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in >> amdgpu_pmu_init(). > [] >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > [] >> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >> case CHIP_VEGA20: >> /* init df */ >> ret = init_pmu_by_type(adev, df_v3_6_attr_groups, >> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> - DF_V3_6_MAX_COUNTERS); >> + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> + DF_V3_6_MAX_COUNTERS); > > trivia: > > The indentation change seems superfluous and > appears to make the code harder to read. > > You could also cc Jonathan Kim who wrote all of this. I think this is just display issue in mail format. It is correct that in vi/vim. The arguments are line up with '(' after my change. @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)$ ^Icase CHIP_VEGA20:$ ^I^I/* init df */$ ^I^Iret = init_pmu_by_type(adev, df_v3_6_attr_groups,$ -^I^I^I^I "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$ -^I^I^I^I DF_V3_6_MAX_COUNTERS);$ +^I^I^I^I^I^I^I "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$ +^I^I^I^I^I^I^I DF_V3_6_MAX_COUNTERS);$ $ ^I^I/* other pmu types go here*/$ ^I^Ibreak;$ > > > > . > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init 2019-06-24 3:41 ` maowenan @ 2019-06-24 3:46 ` Joe Perches 0 siblings, 0 replies; 22+ messages in thread From: Joe Perches @ 2019-06-24 3:46 UTC (permalink / raw) To: maowenan, airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel, Jonathan Kim On Mon, 2019-06-24 at 11:41 +0800, maowenan wrote: > > On 2019/6/23 2:13, Joe Perches wrote: > > On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote: > > > There is one warning: > > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] > > > int ret = 0; > > [] > > > v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in > > > amdgpu_pmu_init(). > > [] > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > [] > > > @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > > > case CHIP_VEGA20: > > > /* init df */ > > > ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > > > - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > > > - DF_V3_6_MAX_COUNTERS); > > > + "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > > > + DF_V3_6_MAX_COUNTERS); > > > > trivia: > > > > The indentation change seems superfluous and > > appears to make the code harder to read. > > > > You could also cc Jonathan Kim who wrote all of this. > I think this is just display issue in mail format. It is correct that in vi/vim. > The arguments are line up with '(' after my change. Use 8 character tabs and try again please. > @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)$ > ^Icase CHIP_VEGA20:$ > ^I^I/* init df */$ > ^I^Iret = init_pmu_by_type(adev, df_v3_6_attr_groups,$ > -^I^I^I^I "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$ > -^I^I^I^I DF_V3_6_MAX_COUNTERS);$ > +^I^I^I^I^I^I^I "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$ > +^I^I^I^I^I^I^I DF_V3_6_MAX_COUNTERS);$ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' 2019-06-22 10:43 ` Dan Carpenter 2019-06-22 13:05 ` [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init Mao Wenan @ 2019-06-23 6:00 ` Dan Carpenter 2019-06-23 6:10 ` Julia Lawall 1 sibling, 1 reply; 22+ messages in thread From: Dan Carpenter @ 2019-06-23 6:00 UTC (permalink / raw) To: Mao Wenan Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, kernel-janitors, amd-gfx, linux-kernel On Sat, Jun 22, 2019 at 01:43:19PM +0300, Dan Carpenter wrote: > On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote: > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > index 0e6dba9..0bf4dd9 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev, > > /* init amdgpu_pmu */ > > int amdgpu_pmu_init(struct amdgpu_device *adev) > > { > > - int ret = 0; > > - > > switch (adev->asic_type) { > > case CHIP_VEGA20: > > /* init df */ > > - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > > + init_pmu_by_type(adev, df_v3_6_attr_groups, > > "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > > DF_V3_6_MAX_COUNTERS); > > > You're resending this for other reasons, but don't forget to update the > indenting on the arguments so they still line up with the '('. > Sorry, I was unclear. If you pull the init_pmu_by_type( back 6 characters then you also need to pull the "DF" back 6 characters. init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS); You can actually fit it into two lines afterwards. regards, dan carpenter ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' 2019-06-23 6:00 ` [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' Dan Carpenter @ 2019-06-23 6:10 ` Julia Lawall 2019-06-24 3:45 ` [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init Mao Wenan 0 siblings, 1 reply; 22+ messages in thread From: Julia Lawall @ 2019-06-23 6:10 UTC (permalink / raw) To: Dan Carpenter Cc: Mao Wenan, airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, kernel-janitors, amd-gfx, linux-kernel On Sun, 23 Jun 2019, Dan Carpenter wrote: > On Sat, Jun 22, 2019 at 01:43:19PM +0300, Dan Carpenter wrote: > > On Sat, Jun 22, 2019 at 11:03:14AM +0800, Mao Wenan wrote: > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > > index 0e6dba9..0bf4dd9 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > > > @@ -246,12 +246,10 @@ static int init_pmu_by_type(struct amdgpu_device *adev, > > > /* init amdgpu_pmu */ > > > int amdgpu_pmu_init(struct amdgpu_device *adev) > > > { > > > - int ret = 0; > > > - > > > switch (adev->asic_type) { > > > case CHIP_VEGA20: > > > /* init df */ > > > - ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > > > + init_pmu_by_type(adev, df_v3_6_attr_groups, > > > "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > > > DF_V3_6_MAX_COUNTERS); > > > > > > You're resending this for other reasons, but don't forget to update the > > indenting on the arguments so they still line up with the '('. > > > > Sorry, I was unclear. If you pull the init_pmu_by_type( back 6 > characters then you also need to pull the "DF" back 6 characters. > > init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df", > PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS); > > You can actually fit it into two lines afterwards. My suggestion was to keep the ret = instead of discarding the indication of failure, so I think that this is not relevant. julia ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init 2019-06-23 6:10 ` Julia Lawall @ 2019-06-24 3:45 ` Mao Wenan 2019-06-24 8:39 ` Dan Carpenter 0 siblings, 1 reply; 22+ messages in thread From: Mao Wenan @ 2019-06-24 3:45 UTC (permalink / raw) To: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel, jonathan.kim, Mao Wenan There is one warning: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] int ret = 0; ^ amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed. amdgpu_device_init() r = amdgpu_pmu_init(adev); This patch is also to update the indenting on the arguments so they line up with the '('. Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") Signed-off-by: Mao Wenan <maowenan@huawei.com> --- v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in amdgpu_pmu_init(). v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 0e6dba9..b702322 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -252,8 +252,11 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) case CHIP_VEGA20: /* init df */ ret = init_pmu_by_type(adev, df_v3_6_attr_groups, - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, - DF_V3_6_MAX_COUNTERS); + "DF", "amdgpu_df", + PERF_TYPE_AMDGPU_DF, + DF_V3_6_MAX_COUNTERS); + if (ret) + return ret; /* other pmu types go here*/ break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init 2019-06-24 3:45 ` [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init Mao Wenan @ 2019-06-24 8:39 ` Dan Carpenter 2019-06-24 9:29 ` maowenan 0 siblings, 1 reply; 22+ messages in thread From: Dan Carpenter @ 2019-06-24 8:39 UTC (permalink / raw) To: Mao Wenan Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, julia.lawall, kernel-janitors, amd-gfx, linux-kernel, jonathan.kim On Mon, Jun 24, 2019 at 11:45:32AM +0800, Mao Wenan wrote: > There is one warning: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] > int ret = 0; > ^ > amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, > which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed. > amdgpu_device_init() > r = amdgpu_pmu_init(adev); > > This patch is also to update the indenting on the arguments so they line up with the '('. > > Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") > > Signed-off-by: Mao Wenan <maowenan@huawei.com> > --- > v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in > amdgpu_pmu_init(). > v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > index 0e6dba9..b702322 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > @@ -252,8 +252,11 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > case CHIP_VEGA20: > /* init df */ > ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > - DF_V3_6_MAX_COUNTERS); > + "DF", "amdgpu_df", > + PERF_TYPE_AMDGPU_DF, > + DF_V3_6_MAX_COUNTERS); > + if (ret) > + return ret; No no. Sorry, the original indenting was correct and lined up with the '(' character in 'init_pmu_by_type(', that's the way it should be. If we were to remove the "ret = " then we'd have to pull the arguments back as well. I think this fix that Julia suggested is really the right so leave the indenting alone. It looks like you've right aligned the arguments. That's not the right way, the original was correct. regards, dan carpenter ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init 2019-06-24 8:39 ` Dan Carpenter @ 2019-06-24 9:29 ` maowenan 2019-06-24 9:48 ` Dan Carpenter 0 siblings, 1 reply; 22+ messages in thread From: maowenan @ 2019-06-24 9:29 UTC (permalink / raw) To: Dan Carpenter Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, julia.lawall, kernel-janitors, amd-gfx, linux-kernel, jonathan.kim, Joe Perches On 2019/6/24 16:39, Dan Carpenter wrote: > On Mon, Jun 24, 2019 at 11:45:32AM +0800, Mao Wenan wrote: >> There is one warning: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] >> int ret = 0; >> ^ >> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, >> which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed. >> amdgpu_device_init() >> r = amdgpu_pmu_init(adev); >> >> This patch is also to update the indenting on the arguments so they line up with the '('. >> >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") >> >> Signed-off-by: Mao Wenan <maowenan@huawei.com> >> --- >> v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in >> amdgpu_pmu_init(). >> v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). >> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> index 0e6dba9..b702322 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> @@ -252,8 +252,11 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >> case CHIP_VEGA20: >> /* init df */ >> ret = init_pmu_by_type(adev, df_v3_6_attr_groups, >> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> - DF_V3_6_MAX_COUNTERS); >> + "DF", "amdgpu_df", >> + PERF_TYPE_AMDGPU_DF, >> + DF_V3_6_MAX_COUNTERS); >> + if (ret) >> + return ret; > > No no. Sorry, the original indenting was correct and lined up with the > '(' character in 'init_pmu_by_type(', that's the way it should be. If > we were to remove the "ret = " then we'd have to pull the arguments back > as well. I think this fix that Julia suggested is really the right so > leave the indenting alone. > > It looks like you've right aligned the arguments. That's not the right > way, the original was correct. > After using 8 character for tab(thanks to Joe), the aligned here is wrong, yes, the original was correct. so my v4 is only to change ret, don't change the indenting? > regards, > dan carpenter > > > . > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init 2019-06-24 9:29 ` maowenan @ 2019-06-24 9:48 ` Dan Carpenter 2019-06-24 11:23 ` [PATCH -next v4] " Mao Wenan 0 siblings, 1 reply; 22+ messages in thread From: Dan Carpenter @ 2019-06-24 9:48 UTC (permalink / raw) To: maowenan Cc: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, julia.lawall, kernel-janitors, amd-gfx, linux-kernel, jonathan.kim, Joe Perches On Mon, Jun 24, 2019 at 05:29:33PM +0800, maowenan wrote: > > > On 2019/6/24 16:39, Dan Carpenter wrote: > > On Mon, Jun 24, 2019 at 11:45:32AM +0800, Mao Wenan wrote: > >> There is one warning: > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] > >> int ret = 0; > >> ^ > >> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, > >> which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed. > >> amdgpu_device_init() > >> r = amdgpu_pmu_init(adev); > >> > >> This patch is also to update the indenting on the arguments so they line up with the '('. > >> > >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") > >> > >> Signed-off-by: Mao Wenan <maowenan@huawei.com> > >> --- > >> v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in > >> amdgpu_pmu_init(). > >> v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). > >> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 7 +++++-- > >> 1 file changed, 5 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> index 0e6dba9..b702322 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > >> @@ -252,8 +252,11 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > >> case CHIP_VEGA20: > >> /* init df */ > >> ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > >> - "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > >> - DF_V3_6_MAX_COUNTERS); > >> + "DF", "amdgpu_df", > >> + PERF_TYPE_AMDGPU_DF, > >> + DF_V3_6_MAX_COUNTERS); > >> + if (ret) > >> + return ret; > > > > No no. Sorry, the original indenting was correct and lined up with the > > '(' character in 'init_pmu_by_type(', that's the way it should be. If > > we were to remove the "ret = " then we'd have to pull the arguments back > > as well. I think this fix that Julia suggested is really the right so > > leave the indenting alone. > > > > > It looks like you've right aligned the arguments. That's not the right > > way, the original was correct. > > > After using 8 character for tab(thanks to Joe), the aligned here is wrong, yes, the original was correct. > > so my v4 is only to change ret, don't change the indenting? > Yes, please. Sorry for my confusing email earlier. regards, dan carpenter ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init 2019-06-24 9:48 ` Dan Carpenter @ 2019-06-24 11:23 ` Mao Wenan 2019-06-24 17:42 ` Kim, Jonathan 0 siblings, 1 reply; 22+ messages in thread From: Mao Wenan @ 2019-06-24 11:23 UTC (permalink / raw) To: airlied, daniel, alexander.deucher, christian.koenig, David1.Zhou, dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel, jonathan.kim, Mao Wenan There is one warning: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] int ret = 0; ^ amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed. amdgpu_device_init() r = amdgpu_pmu_init(adev); Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") Signed-off-by: Mao Wenan <maowenan@huawei.com> --- v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in amdgpu_pmu_init(). v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). v3->v4: delete the indenting for init_pmu_by_type() arguments. The original indenting is correct. drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 0e6dba9f60f0..c98cf77a37f3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -254,6 +254,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) ret = init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS); + if (ret) + return ret; /* other pmu types go here*/ break; -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* RE: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init 2019-06-24 11:23 ` [PATCH -next v4] " Mao Wenan @ 2019-06-24 17:42 ` Kim, Jonathan 2019-06-26 11:35 ` maowenan 0 siblings, 1 reply; 22+ messages in thread From: Kim, Jonathan @ 2019-06-24 17:42 UTC (permalink / raw) To: Mao Wenan, airlied, daniel, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel Immediate return should be ok since perf registration isn't dependent on gpu hw. Reviewed-by: Jonathan Kim <Jonathan.Kim@amd.com> -----Original Message----- From: Mao Wenan <maowenan@huawei.com> Sent: Monday, June 24, 2019 7:23 AM To: airlied@linux.ie; daniel@ffwll.ch; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>; dan.carpenter@oracle.com; julia.lawall@lip6.fr Cc: kernel-janitors@vger.kernel.org; amd-gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org; Kim, Jonathan <Jonathan.Kim@amd.com>; Mao Wenan <maowenan@huawei.com> Subject: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init [CAUTION: External Email] There is one warning: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] int ret = 0; ^ amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed. amdgpu_device_init() r = amdgpu_pmu_init(adev); Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") Signed-off-by: Mao Wenan <maowenan@huawei.com> --- v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in amdgpu_pmu_init(). v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). v3->v4: delete the indenting for init_pmu_by_type() arguments. The original indenting is correct. drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c index 0e6dba9f60f0..c98cf77a37f3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c @@ -254,6 +254,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) ret = init_pmu_by_type(adev, df_v3_6_attr_groups, "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, DF_V3_6_MAX_COUNTERS); + if (ret) + return ret; /* other pmu types go here*/ break; -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init 2019-06-24 17:42 ` Kim, Jonathan @ 2019-06-26 11:35 ` maowenan 2019-07-10 7:31 ` maowenan 0 siblings, 1 reply; 22+ messages in thread From: maowenan @ 2019-06-26 11:35 UTC (permalink / raw) To: Kim, Jonathan, airlied, daniel, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel On 2019/6/25 1:42, Kim, Jonathan wrote: > Immediate return should be ok since perf registration isn't dependent on gpu hw. > > Reviewed-by: Jonathan Kim <Jonathan.Kim@amd.com> thanks for review. > > -----Original Message----- > From: Mao Wenan <maowenan@huawei.com> > Sent: Monday, June 24, 2019 7:23 AM > To: airlied@linux.ie; daniel@ffwll.ch; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>; dan.carpenter@oracle.com; julia.lawall@lip6.fr > Cc: kernel-janitors@vger.kernel.org; amd-gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org; Kim, Jonathan <Jonathan.Kim@amd.com>; Mao Wenan <maowenan@huawei.com> > Subject: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init > > [CAUTION: External Email] > > There is one warning: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] > int ret = 0; > ^ > amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, > which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed. > amdgpu_device_init() > r = amdgpu_pmu_init(adev); > > Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") > > Signed-off-by: Mao Wenan <maowenan@huawei.com> > --- > v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in amdgpu_pmu_init(). > v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). > v3->v4: delete the indenting for init_pmu_by_type() arguments. The original indenting is correct. > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > index 0e6dba9f60f0..c98cf77a37f3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > @@ -254,6 +254,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > ret = init_pmu_by_type(adev, df_v3_6_attr_groups, > "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, > DF_V3_6_MAX_COUNTERS); > + if (ret) > + return ret; > > /* other pmu types go here*/ > break; > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init 2019-06-26 11:35 ` maowenan @ 2019-07-10 7:31 ` maowenan 0 siblings, 0 replies; 22+ messages in thread From: maowenan @ 2019-07-10 7:31 UTC (permalink / raw) To: Kim, Jonathan, airlied, daniel, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), dan.carpenter, julia.lawall Cc: kernel-janitors, amd-gfx, linux-kernel gentle ping On 2019/6/26 19:35, maowenan wrote: > > > On 2019/6/25 1:42, Kim, Jonathan wrote: >> Immediate return should be ok since perf registration isn't dependent on gpu hw. >> >> Reviewed-by: Jonathan Kim <Jonathan.Kim@amd.com> > > thanks for review. > >> >> -----Original Message----- >> From: Mao Wenan <maowenan@huawei.com> >> Sent: Monday, June 24, 2019 7:23 AM >> To: airlied@linux.ie; daniel@ffwll.ch; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing) <David1.Zhou@amd.com>; dan.carpenter@oracle.com; julia.lawall@lip6.fr >> Cc: kernel-janitors@vger.kernel.org; amd-gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org; Kim, Jonathan <Jonathan.Kim@amd.com>; Mao Wenan <maowenan@huawei.com> >> Subject: [PATCH -next v4] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init >> >> [CAUTION: External Email] >> >> There is one warning: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’: >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable] >> int ret = 0; >> ^ >> amdgpu_pmu_init() is called by amdgpu_device_init() in drivers/gpu/drm/amd/amdgpu/amdgpu_device.c, >> which will use the return value. So it should return 'ret' immediately if init_pmu_by_type() failed. >> amdgpu_device_init() >> r = amdgpu_pmu_init(adev); >> >> Fixes: 9c7c85f7ea1f ("drm/amdgpu: add pmu counters") >> >> Signed-off-by: Mao Wenan <maowenan@huawei.com> >> --- >> v1->v2: change the subject for this patch; change the indenting when it calls init_pmu_by_type; use the value 'ret' in amdgpu_pmu_init(). >> v2->v3: change the subject for this patch; return 'ret' immediately if failed to call init_pmu_by_type(). >> v3->v4: delete the indenting for init_pmu_by_type() arguments. The original indenting is correct. >> >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> index 0e6dba9f60f0..c98cf77a37f3 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c >> @@ -254,6 +254,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) >> ret = init_pmu_by_type(adev, df_v3_6_attr_groups, >> "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF, >> DF_V3_6_MAX_COUNTERS); >> + if (ret) >> + return ret; >> >> /* other pmu types go here*/ >> break; >> -- >> 2.20.1 >> > > > . > ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-07-10 7:31 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-22 3:03 [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' Mao Wenan 2019-06-22 6:02 ` Julia Lawall 2019-06-22 7:20 ` maowenan 2019-06-22 10:43 ` Dan Carpenter 2019-06-22 13:05 ` [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init Mao Wenan 2019-06-22 13:06 ` Julia Lawall 2019-06-22 13:56 ` maowenan 2019-06-22 14:00 ` Julia Lawall 2019-06-24 3:16 ` maowenan 2019-06-22 18:13 ` Joe Perches 2019-06-24 3:41 ` maowenan 2019-06-24 3:46 ` Joe Perches 2019-06-23 6:00 ` [PATCH -next] drm/amdgpu: remove set but not used variables 'ret' Dan Carpenter 2019-06-23 6:10 ` Julia Lawall 2019-06-24 3:45 ` [PATCH -next v3] drm/amdgpu: return 'ret' immediately if failed in amdgpu_pmu_init Mao Wenan 2019-06-24 8:39 ` Dan Carpenter 2019-06-24 9:29 ` maowenan 2019-06-24 9:48 ` Dan Carpenter 2019-06-24 11:23 ` [PATCH -next v4] " Mao Wenan 2019-06-24 17:42 ` Kim, Jonathan 2019-06-26 11:35 ` maowenan 2019-07-10 7:31 ` maowenan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).