[PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[Kevin 'ldir' Darbyshire-Bryant]

Turns out tin_quantum_prio isn't used anymore and is a leftover from a
previous implementation of diffserv tins.  Since the variable isn't used
in any calculations it can be eliminated.

Drop variable and places where it was set.  Rename remaining variable
and consolidate naming of intermediate variables that set it.

Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
---
v2 - rename intermediate 'quantum1' variables to just 'quantum' since
there's only one of them left now.  Also rename remaining
tin_quantum_band to tin_quantum since there's now only 1 type of
quantum.

Please drop the v1 patch

 net/sched/sch_cake.c | 59 ++++++++++++++------------------------------
 1 file changed, 18 insertions(+), 41 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 53a80bc6b13a..9ce85b97169c 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -173,8 +173,7 @@ struct cake_tin_data {
 	u64	tin_rate_bps;
 	u16	tin_rate_shft;
 
-	u16	tin_quantum_prio;
-	u16	tin_quantum_band;
+	u16	tin_quantum;
 	s32	tin_deficit;
 	u32	tin_backlog;
 	u32	tin_dropped;
@@ -1919,7 +1918,7 @@ static struct sk_buff *cake_dequeue(struct Qdisc *sch)
 		while (b->tin_deficit < 0 ||
 		       !(b->sparse_flow_count + b->bulk_flow_count)) {
 			if (b->tin_deficit <= 0)
-				b->tin_deficit += b->tin_quantum_band;
+				b->tin_deficit += b->tin_quantum;
 			if (b->sparse_flow_count + b->bulk_flow_count)
 				empty = false;
 
@@ -2240,8 +2239,7 @@ static int cake_config_besteffort(struct Qdisc *sch)
 
 	cake_set_rate(b, rate, mtu,
 		      us_to_ns(q->target), us_to_ns(q->interval));
-	b->tin_quantum_band = 65535;
-	b->tin_quantum_prio = 65535;
+	b->tin_quantum = 65535;
 
 	return 0;
 }
@@ -2252,8 +2250,7 @@ static int cake_config_precedence(struct Qdisc *sch)
 	struct cake_sched_data *q = qdisc_priv(sch);
 	u32 mtu = psched_mtu(qdisc_dev(sch));
 	u64 rate = q->rate_bps;
-	u32 quantum1 = 256;
-	u32 quantum2 = 256;
+	u32 quantum = 256;
 	u32 i;
 
 	q->tin_cnt = 8;
@@ -2266,18 +2263,14 @@ static int cake_config_precedence(struct Qdisc *sch)
 		cake_set_rate(b, rate, mtu, us_to_ns(q->target),
 			      us_to_ns(q->interval));
 
-		b->tin_quantum_prio = max_t(u16, 1U, quantum1);
-		b->tin_quantum_band = max_t(u16, 1U, quantum2);
+		b->tin_quantum = max_t(u16, 1U, quantum);
 
 		/* calculate next class's parameters */
 		rate  *= 7;
 		rate >>= 3;
 
-		quantum1  *= 3;
-		quantum1 >>= 1;
-
-		quantum2  *= 7;
-		quantum2 >>= 3;
+		quantum  *= 7;
+		quantum >>= 3;
 	}
 
 	return 0;
@@ -2346,8 +2339,7 @@ static int cake_config_diffserv8(struct Qdisc *sch)
 	struct cake_sched_data *q = qdisc_priv(sch);
 	u32 mtu = psched_mtu(qdisc_dev(sch));
 	u64 rate = q->rate_bps;
-	u32 quantum1 = 256;
-	u32 quantum2 = 256;
+	u32 quantum = 256;
 	u32 i;
 
 	q->tin_cnt = 8;
@@ -2363,18 +2355,14 @@ static int cake_config_diffserv8(struct Qdisc *sch)
 		cake_set_rate(b, rate, mtu, us_to_ns(q->target),
 			      us_to_ns(q->interval));
 
-		b->tin_quantum_prio = max_t(u16, 1U, quantum1);
-		b->tin_quantum_band = max_t(u16, 1U, quantum2);
+		b->tin_quantum = max_t(u16, 1U, quantum);
 
 		/* calculate next class's parameters */
 		rate  *= 7;
 		rate >>= 3;
 
-		quantum1  *= 3;
-		quantum1 >>= 1;
-
-		quantum2  *= 7;
-		quantum2 >>= 3;
+		quantum  *= 7;
+		quantum >>= 3;
 	}
 
 	return 0;
@@ -2413,17 +2401,11 @@ static int cake_config_diffserv4(struct Qdisc *sch)
 	cake_set_rate(&q->tins[3], rate >> 2, mtu,
 		      us_to_ns(q->target), us_to_ns(q->interval));
 
-	/* priority weights */
-	q->tins[0].tin_quantum_prio = quantum;
-	q->tins[1].tin_quantum_prio = quantum >> 4;
-	q->tins[2].tin_quantum_prio = quantum << 2;
-	q->tins[3].tin_quantum_prio = quantum << 4;
-
 	/* bandwidth-sharing weights */
-	q->tins[0].tin_quantum_band = quantum;
-	q->tins[1].tin_quantum_band = quantum >> 4;
-	q->tins[2].tin_quantum_band = quantum >> 1;
-	q->tins[3].tin_quantum_band = quantum >> 2;
+	q->tins[0].tin_quantum = quantum;
+	q->tins[1].tin_quantum = quantum >> 4;
+	q->tins[2].tin_quantum = quantum >> 1;
+	q->tins[3].tin_quantum = quantum >> 2;
 
 	return 0;
 }
@@ -2454,15 +2436,10 @@ static int cake_config_diffserv3(struct Qdisc *sch)
 	cake_set_rate(&q->tins[2], rate >> 2, mtu,
 		      us_to_ns(q->target), us_to_ns(q->interval));
 
-	/* priority weights */
-	q->tins[0].tin_quantum_prio = quantum;
-	q->tins[1].tin_quantum_prio = quantum >> 4;
-	q->tins[2].tin_quantum_prio = quantum << 4;
-
 	/* bandwidth-sharing weights */
-	q->tins[0].tin_quantum_band = quantum;
-	q->tins[1].tin_quantum_band = quantum >> 4;
-	q->tins[2].tin_quantum_band = quantum >> 2;
+	q->tins[0].tin_quantum = quantum;
+	q->tins[1].tin_quantum = quantum >> 4;
+	q->tins[2].tin_quantum = quantum >> 2;
 
 	return 0;
 }
-- 
2.21.0 (Apple Git-122.2)

Re: [PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[Toke Høiland-Jørgensen]

Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> writes:

> Turns out tin_quantum_prio isn't used anymore and is a leftover from a
> previous implementation of diffserv tins.  Since the variable isn't used
> in any calculations it can be eliminated.
>
> Drop variable and places where it was set.  Rename remaining variable
> and consolidate naming of intermediate variables that set it.
>
> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>

Re: [PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[Jakub Kicinski]

On Wed, 18 Dec 2019 14:05:13 +0000, Kevin 'ldir' Darbyshire-Bryant
wrote:
> Turns out tin_quantum_prio isn't used anymore and is a leftover from a
> previous implementation of diffserv tins.  Since the variable isn't used
> in any calculations it can be eliminated.
> 
> Drop variable and places where it was set.  Rename remaining variable
> and consolidate naming of intermediate variables that set it.
> 
> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>

Checkpatch sayeth:

WARNING: Missing Signed-off-by: line by nominal patch author 'Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>'

total: 0 errors, 1 warnings, 0 checks, 125 lines checked

Re: [PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[David Miller]

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed, 18 Dec 2019 09:53:40 -0800

> On Wed, 18 Dec 2019 14:05:13 +0000, Kevin 'ldir' Darbyshire-Bryant
> wrote:
>> Turns out tin_quantum_prio isn't used anymore and is a leftover from a
>> previous implementation of diffserv tins.  Since the variable isn't used
>> in any calculations it can be eliminated.
>> 
>> Drop variable and places where it was set.  Rename remaining variable
>> and consolidate naming of intermediate variables that set it.
>> 
>> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
> 
> Checkpatch sayeth:
> 
> WARNING: Missing Signed-off-by: line by nominal patch author 'Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>'

Which is kinda rediculous wouldn't you say? :-)

The warning stops to be useful if it's going to be applied in situations
like this where merely a nickname 'ldir' is added to the middle of the
person's formal name.

I would never push back on a patch on these grounds, it just wastes time.

Re: [PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[Kevin 'ldir' Darbyshire-Bryant]

[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]



> On 18 Dec 2019, at 20:37, David Miller <davem@davemloft.net> wrote:
> 
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Wed, 18 Dec 2019 09:53:40 -0800
> 
>> On Wed, 18 Dec 2019 14:05:13 +0000, Kevin 'ldir' Darbyshire-Bryant
>> wrote:
>>> Turns out tin_quantum_prio isn't used anymore and is a leftover from a
>>> previous implementation of diffserv tins.  Since the variable isn't used
>>> in any calculations it can be eliminated.
>>> 
>>> Drop variable and places where it was set.  Rename remaining variable
>>> and consolidate naming of intermediate variables that set it.
>>> 
>>> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
>> 
>> Checkpatch sayeth:
>> 
>> WARNING: Missing Signed-off-by: line by nominal patch author 'Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>'
> 
> Which is kinda rediculous wouldn't you say? :-)
> 
> The warning stops to be useful if it's going to be applied in situations
> like this where merely a nickname 'ldir' is added to the middle of the
> person's formal name.
> 
> I would never push back on a patch on these grounds, it just wastes time.

Hi David,

Thanks for that.  It has prompted confusion/investigation at my end to work
why exchange is inserting the alias into the from field though!  Think I’ve
worked that out and will try plan B on the next patch submission.

:-)

Kevin

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[David Miller]

From: Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
Date: Wed, 18 Dec 2019 14:05:13 +0000

> Turns out tin_quantum_prio isn't used anymore and is a leftover from a
> previous implementation of diffserv tins.  Since the variable isn't used
> in any calculations it can be eliminated.
> 
> Drop variable and places where it was set.  Rename remaining variable
> and consolidate naming of intermediate variables that set it.
> 
> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>

Applied, thank you.

Re: [PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[Kevin 'ldir' Darbyshire-Bryant]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]



> On 18 Dec 2019, at 21:27, David Miller <davem@davemloft.net> wrote:
> 
> From: Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
> Date: Wed, 18 Dec 2019 14:05:13 +0000
> 
>> Turns out tin_quantum_prio isn't used anymore and is a leftover from a
>> previous implementation of diffserv tins.  Since the variable isn't used
>> in any calculations it can be eliminated.
>> 
>> Drop variable and places where it was set.  Rename remaining variable
>> and consolidate naming of intermediate variables that set it.
>> 
>> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
> 
> Applied, thank you.

Wow, thank you!

It’s a great feeling getting a patch into the kernel :-)

Cheers,

Kevin D-B

gpg: 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next v2] sch_cake: drop unused variable tin_quantum_prio

[Jakub Kicinski]

On Wed, 18 Dec 2019 12:37:16 -0800 (PST), David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Wed, 18 Dec 2019 09:53:40 -0800
> 
> > On Wed, 18 Dec 2019 14:05:13 +0000, Kevin 'ldir' Darbyshire-Bryant
> > wrote:  
> >> Turns out tin_quantum_prio isn't used anymore and is a leftover from a
> >> previous implementation of diffserv tins.  Since the variable isn't used
> >> in any calculations it can be eliminated.
> >> 
> >> Drop variable and places where it was set.  Rename remaining variable
> >> and consolidate naming of intermediate variables that set it.
> >> 
> >> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>  
> > 
> > Checkpatch sayeth:
> > 
> > WARNING: Missing Signed-off-by: line by nominal patch author 'Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>'  
> 
> Which is kinda rediculous wouldn't you say? :-)
> 
> The warning stops to be useful if it's going to be applied in situations
> like this where merely a nickname 'ldir' is added to the middle of the
> person's formal name.
> 
> I would never push back on a patch on these grounds, it just wastes time.

Yup, just tuning the checks, this one I wasn't 100% sure :-)

Looks like Greg's script only complains if both name and address are
different, but checkpatch expects the exact same thing. I'll stick to
Greg's method.

Feedback is very welcome :-)