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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
>
>
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 <[email protected]>
>> ---
>> 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
>>
>>
>
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
>
>
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 <[email protected]>
>> ---
>> 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
>>
>>
>
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 <[email protected]>
> >> ---
> >> 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
> >>
> >>
> >
>
>
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.
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
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
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 <[email protected]>
>>>> ---
>>>> 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
>>>>
>>>>
>>>
>>
>>
>
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 <[email protected]>
---
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
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;$
>
>
>
> .
>
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);$
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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
>
>
> .
>
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 <[email protected]>
> >> ---
> >> 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
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 <[email protected]>
---
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
Immediate return should be ok since perf registration isn't dependent on gpu hw.
Reviewed-by: Jonathan Kim <[email protected]>
-----Original Message-----
From: Mao Wenan <[email protected]>
Sent: Monday, June 24, 2019 7:23 AM
To: [email protected]; [email protected]; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Zhou, David(ChunMing) <[email protected]>; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; Kim, Jonathan <[email protected]>; Mao Wenan <[email protected]>
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 <[email protected]>
---
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
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 <[email protected]>
thanks for review.
>
> -----Original Message-----
> From: Mao Wenan <[email protected]>
> Sent: Monday, June 24, 2019 7:23 AM
> To: [email protected]; [email protected]; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Zhou, David(ChunMing) <[email protected]>; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; Kim, Jonathan <[email protected]>; Mao Wenan <[email protected]>
> 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 <[email protected]>
> ---
> 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
>
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 <[email protected]>
>
> thanks for review.
>
>>
>> -----Original Message-----
>> From: Mao Wenan <[email protected]>
>> Sent: Monday, June 24, 2019 7:23 AM
>> To: [email protected]; [email protected]; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Zhou, David(ChunMing) <[email protected]>; [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; Kim, Jonathan <[email protected]>; Mao Wenan <[email protected]>
>> 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 <[email protected]>
>> ---
>> 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
>>
>
>
> .
>