netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paolo valente <paolo.valente@unimore.it>
To: Jamal Hadi Salim <jhs@mojatatu.com>,
	"David S. Miller" <davem@davemloft.net>,
	shemminger@vyatta.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	fchecconi@gmail.com, rizzo@iet.unipi.it,
	Paolo Valente <paolo.valente@unimore.it>
Subject: [PATCH BUGFIX 1/6] pkt_sched: properly cap timestamps in charge_actual_service
Date: Tue,  5 Mar 2013 19:04:57 +0100	[thread overview]
Message-ID: <1362506702-4985-2-git-send-email-paolo.valente@unimore.it> (raw)
In-Reply-To: <1362506702-4985-1-git-send-email-paolo.valente@unimore.it>

QFQ+ schedules the active aggregates in a group using a bucket list
(one list per group). The bucket in which each aggregate is inserted
depends on the aggregate's timestamps, and the number
of buckets in a group is enough to accomodate the possible (range of)
values of the timestamps of all the aggregates in the group. For this
property to hold, timestamps must however be computed correctly.  One
necessary condition for computing timestamps correctly is that the
number of bits dequeued for each aggregate, while the aggregate is in
service, does not exceed the maximum budget budgetmax assigned to the
aggregate.

For each aggregate, budgetmax is proportional to the number of classes
in the aggregate. If the number of classes of the aggregate is
decreased through qfq_change_class(), then budgetmax is decreased
automatically as well.  Problems may occur if the aggregate is in
service when budgetmax is decreased, because the current remaining
budget of the aggregate and/or the service already received by the
aggregate may happen to be larger than the new value of budgetmax.  In
this case, when the aggregate is eventually deselected and its
timestamps are updated, the aggregate may happen to have received an
amount of service larger than budgetmax.  This may cause the aggregate
to be assigned a higher virtual finish time than the maximum
acceptable value for the last bucket in the bucket list of the group.

This fix introduces a cap that addresses this issue.

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

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 6ed3765..0f6e2db 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -299,6 +299,10 @@ static void qfq_update_agg(struct qfq_sched *q, struct qfq_aggregate *agg,
 	    new_num_classes == q->max_agg_classes - 1) /* agg no more full */
 		hlist_add_head(&agg->nonfull_next, &q->nonfull_aggs);
 
+	/* The next assignment may let
+	 * agg->initial_budget > agg->budgetmax
+	 * hold, we will take it into account in charge_actual_service().
+	 */
 	agg->budgetmax = new_num_classes * agg->lmax;
 	new_agg_weight = agg->class_weight * new_num_classes;
 	agg->inv_w = ONE_FP/new_agg_weight;
@@ -990,8 +994,13 @@ static inline struct sk_buff *qfq_peek_skb(struct qfq_aggregate *agg,
 /* Update F according to the actual service received by the aggregate. */
 static inline void charge_actual_service(struct qfq_aggregate *agg)
 {
-	/* compute the service received by the aggregate */
-	u32 service_received = agg->initial_budget - agg->budget;
+	/* Compute the service received by the aggregate, taking into
+	 * account that, after decreasing the number of classes in
+	 * agg, it may happen that
+	 * agg->initial_budget - agg->budget > agg->bugdetmax
+	 */
+	u32 service_received = min(agg->budgetmax,
+				   agg->initial_budget - agg->budget);
 
 	agg->F = agg->S + (u64)service_received * agg->inv_w;
 }
-- 
1.7.9.5

  reply	other threads:[~2013-03-05 18:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-19 17:31 [PATCH BUGFIX] pkt_sched: fix little service anomalies and possible crashes of qfq+ Paolo valente
2012-12-26 23:13 ` David Miller
2013-02-26 17:02   ` Paolo valente
2013-02-26 22:37     ` David Miller
2013-03-05 18:04       ` [PATCH BUGFIX 0/6] " Paolo valente
2013-03-05 18:04         ` Paolo valente [this message]
2013-03-05 18:04         ` [PATCH BUGFIX 2/6] pkt_sched: fix the update of eligible-group sets Paolo valente
2013-03-06 10:05           ` David Laight
2013-03-05 18:04         ` [PATCH BUGFIX 3/6] pkt_sched: serve activated aggregates immediately if the scheduler is empty Paolo valente
2013-03-05 18:05         ` [PATCH BUGFIX 4/6] pkt_sched: prevent budget from wrapping around after a dequeue Paolo valente
2013-03-05 18:05         ` [PATCH BUGFIX 5/6] pkt_sched: do not allow virtual time to jump if an aggregate is in service Paolo valente
2013-03-05 18:05         ` [PATCH BUGFIX 6/6] pkt_sched: remove a useless invocation of qfq_update_eligible Paolo valente
2013-03-06  4:50         ` [PATCH BUGFIX 0/6] pkt_sched: fix little service anomalies and possible crashes of qfq+ David Miller
2013-07-10 13:46           ` [PATCH 0/2] pkt_sched: sch_qfq: efficiency and codestyle improvements Paolo Valente
2013-07-10 13:46             ` [PATCH 1/2] pkt_sched: sch_qfq: improve efficiency of make_eligible Paolo Valente
2013-07-10 13:46             ` [PATCH 2/2] pkt_sched: sch_qfq: remove forward declaration of qfq_update_agg_ts Paolo Valente
2013-07-11 20:01             ` [PATCH 0/2] pkt_sched: sch_qfq: efficiency and codestyle improvements David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1362506702-4985-2-git-send-email-paolo.valente@unimore.it \
    --to=paolo.valente@unimore.it \
    --cc=davem@davemloft.net \
    --cc=fchecconi@gmail.com \
    --cc=jhs@mojatatu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rizzo@iet.unipi.it \
    --cc=shemminger@vyatta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).