2019-08-08 15:00:31

by Yue Haibing

[permalink] [raw]
Subject: [PATCH net-next] taprio: remove unused variable 'entry_list_policy'

net/sched/sch_taprio.c:680:32: warning:
entry_list_policy defined but not used [-Wunused-const-variable=]

It is not used since commit a3d43c0d56f1 ("taprio: Add
support adding an admin schedule")

Reported-by: Hulk Robot <[email protected]>
Signed-off-by: YueHaibing <[email protected]>
---
net/sched/sch_taprio.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index c39db50..046fd2c 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -677,10 +677,6 @@ static const struct nla_policy entry_policy[TCA_TAPRIO_SCHED_ENTRY_MAX + 1] = {
[TCA_TAPRIO_SCHED_ENTRY_INTERVAL] = { .type = NLA_U32 },
};

-static const struct nla_policy entry_list_policy[TCA_TAPRIO_SCHED_MAX + 1] = {
- [TCA_TAPRIO_SCHED_ENTRY] = { .type = NLA_NESTED },
-};
-
static const struct nla_policy taprio_policy[TCA_TAPRIO_ATTR_MAX + 1] = {
[TCA_TAPRIO_ATTR_PRIOMAP] = {
.len = sizeof(struct tc_mqprio_qopt)
--
2.7.4



2019-08-08 20:34:09

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next] taprio: remove unused variable 'entry_list_policy'

From: YueHaibing <[email protected]>
Date: Thu, 8 Aug 2019 22:26:23 +0800

> net/sched/sch_taprio.c:680:32: warning:
> entry_list_policy defined but not used [-Wunused-const-variable=]
>
> It is not used since commit a3d43c0d56f1 ("taprio: Add
> support adding an admin schedule")
>
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: YueHaibing <[email protected]>

This is probably unintentional and a bug, we should be using that
policy value to validate that the sched list is indeed a nested
attribute.

I'm not applying this without at least a better and clear commit
message explaining why we shouldn't be using this policy any more.

2019-08-08 21:00:25

by Vinicius Costa Gomes

[permalink] [raw]
Subject: Re: [PATCH net-next] taprio: remove unused variable 'entry_list_policy'

Hi,

David Miller <[email protected]> writes:

> From: YueHaibing <[email protected]>
> Date: Thu, 8 Aug 2019 22:26:23 +0800
>
>> net/sched/sch_taprio.c:680:32: warning:
>> entry_list_policy defined but not used [-Wunused-const-variable=]
>>
>> It is not used since commit a3d43c0d56f1 ("taprio: Add
>> support adding an admin schedule")
>>
>> Reported-by: Hulk Robot <[email protected]>
>> Signed-off-by: YueHaibing <[email protected]>
>
> This is probably unintentional and a bug, we should be using that
> policy value to validate that the sched list is indeed a nested
> attribute.

Removing this policy should be fine.

One of the points of commit (as explained in the commit message)
a3d43c0d56f1 ("taprio: Add support adding an admin schedule") is that it
removes support (it now returns "not supported") for schedules using the
TCA_TAPRIO_ATTR_SCHED_SINGLE_ENTRY attribute (which were never used),
the parsing of those types of schedules was the only user of this
policy.

>
> I'm not applying this without at least a better and clear commit
> message explaining why we shouldn't be using this policy any more.

YueHaibing may use the text above in the commit message of a new spin of
this patch if you think it's clear enough.


Cheers,
--
Vinicius

2019-08-09 01:34:02

by Yue Haibing

[permalink] [raw]
Subject: Re: [PATCH net-next] taprio: remove unused variable 'entry_list_policy'

On 2019/8/9 4:42, Vinicius Costa Gomes wrote:
> Hi,
>
> David Miller <[email protected]> writes:
>
>> From: YueHaibing <[email protected]>
>> Date: Thu, 8 Aug 2019 22:26:23 +0800
>>
>>> net/sched/sch_taprio.c:680:32: warning:
>>> entry_list_policy defined but not used [-Wunused-const-variable=]
>>>
>>> It is not used since commit a3d43c0d56f1 ("taprio: Add
>>> support adding an admin schedule")
>>>
>>> Reported-by: Hulk Robot <[email protected]>
>>> Signed-off-by: YueHaibing <[email protected]>
>>
>> This is probably unintentional and a bug, we should be using that
>> policy value to validate that the sched list is indeed a nested
>> attribute.
>
> Removing this policy should be fine.
>
> One of the points of commit (as explained in the commit message)
> a3d43c0d56f1 ("taprio: Add support adding an admin schedule") is that it
> removes support (it now returns "not supported") for schedules using the
> TCA_TAPRIO_ATTR_SCHED_SINGLE_ENTRY attribute (which were never used),
> the parsing of those types of schedules was the only user of this
> policy.
>
>>
>> I'm not applying this without at least a better and clear commit
>> message explaining why we shouldn't be using this policy any more.
>
> YueHaibing may use the text above in the commit message of a new spin of
> this patch if you think it's clear enough.

Thanks, will send v2 with your explanation.

>
>
> Cheers,
> --
> Vinicius
>
> .
>

2019-08-09 01:52:17

by Yue Haibing

[permalink] [raw]
Subject: [PATCH v2 net-next] taprio: remove unused variable 'entry_list_policy'

net/sched/sch_taprio.c:680:32: warning:
entry_list_policy defined but not used [-Wunused-const-variable=]

One of the points of commit a3d43c0d56f1 ("taprio: Add support adding
an admin schedule") is that it removes support (it now returns "not
supported") for schedules using the TCA_TAPRIO_ATTR_SCHED_SINGLE_ENTRY
attribute (which were never used), the parsing of those types of schedules
was the only user of this policy. So removing this policy should be fine.

Reported-by: Hulk Robot <[email protected]>
Suggested-by: Vinicius Costa Gomes <[email protected]>
Signed-off-by: YueHaibing <[email protected]>
---
v2: respin commit log using Vinicius's explanation.
---
net/sched/sch_taprio.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index c39db50..046fd2c 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -677,10 +677,6 @@ static const struct nla_policy entry_policy[TCA_TAPRIO_SCHED_ENTRY_MAX + 1] = {
[TCA_TAPRIO_SCHED_ENTRY_INTERVAL] = { .type = NLA_U32 },
};

-static const struct nla_policy entry_list_policy[TCA_TAPRIO_SCHED_MAX + 1] = {
- [TCA_TAPRIO_SCHED_ENTRY] = { .type = NLA_NESTED },
-};
-
static const struct nla_policy taprio_policy[TCA_TAPRIO_ATTR_MAX + 1] = {
[TCA_TAPRIO_ATTR_PRIOMAP] = {
.len = sizeof(struct tc_mqprio_qopt)
--
2.7.4


2019-08-09 20:42:43

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2 net-next] taprio: remove unused variable 'entry_list_policy'

From: YueHaibing <[email protected]>
Date: Fri, 9 Aug 2019 09:49:23 +0800

> net/sched/sch_taprio.c:680:32: warning:
> entry_list_policy defined but not used [-Wunused-const-variable=]
>
> One of the points of commit a3d43c0d56f1 ("taprio: Add support adding
> an admin schedule") is that it removes support (it now returns "not
> supported") for schedules using the TCA_TAPRIO_ATTR_SCHED_SINGLE_ENTRY
> attribute (which were never used), the parsing of those types of schedules
> was the only user of this policy. So removing this policy should be fine.
>
> Reported-by: Hulk Robot <[email protected]>
> Suggested-by: Vinicius Costa Gomes <[email protected]>
> Signed-off-by: YueHaibing <[email protected]>
> ---
> v2: respin commit log using Vinicius's explanation.

Applied.