diff mbox

[BUGFIX,2/6] pkt_sched: fix the update of eligible-group sets

Message ID 1362506702-4985-3-git-send-email-paolo.valente@unimore.it
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Paolo Valente March 5, 2013, 6:04 p.m. UTC 
Between two invocations of make_eligible, the system virtual time may
happen to grow enough that, in its binary representation, a bit with
higher order than 31 flips. This happens especially with
TSO/GSO. Before this fix, the mask used in make_eligible was computed
as (1UL<<index_of_last_flipped_bit)-1, whose value is well defined on
a 64-bit architecture, because index_of_flipped_bit <= 63, but is in
general undefined on a 32-bit architecture if index_of_flipped_bit > 31.
The fix just replaces 1UL with 1ULL.

Signed-off-by: Paolo Valente <paolo.valente@unimore.it>
Reviewed-by: Fabio Checconi <fchecconi@gmail.com>
---
 net/sched/sch_qfq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Laight March 6, 2013, 10:05 a.m. UTC | #1 
> Between two invocations of make_eligible, the system virtual time may
> happen to grow enough that, in its binary representation, a bit with
> higher order than 31 flips. This happens especially with
> TSO/GSO. Before this fix, the mask used in make_eligible was computed
> as (1UL<<index_of_last_flipped_bit)-1, whose value is well defined on
> a 64-bit architecture, because index_of_flipped_bit <= 63, but is in
> general undefined on a 32-bit architecture if index_of_flipped_bit > 31.
> The fix just replaces 1UL with 1ULL.
...
> -		unsigned long mask = (1UL << fls(vslot ^ old_vslot)) - 1;
> +		unsigned long mask = (1ULL << fls(vslot ^ old_vslot)) - 1;

I'm not sure you really want to be doing 64bit shifts on 32 bit
systems just to generate ~0ul.
probably worth a conditional.

	David


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 0f6e2db..4cbbf79 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -823,7 +823,7 @@  static void qfq_make_eligible(struct qfq_sched *q)
 	unsigned long old_vslot = q->oldV >> q->min_slot_shift;
 
 	if (vslot != old_vslot) {
-		unsigned long mask = (1UL << fls(vslot ^ old_vslot)) - 1;
+		unsigned long mask = (1ULL << fls(vslot ^ old_vslot)) - 1;
 		qfq_move_groups(q, mask, IR, ER);
 		qfq_move_groups(q, mask, IB, EB);
 	}