From 0cfd32b736ae0c36b42697584811042726c07cba Mon Sep 17 00:00:00 2001 From: Hiroaki SHIMODA Date: Wed, 30 May 2012 12:24:39 +0000 Subject: bql: Fix POSDIFF() to integer overflow aware. POSDIFF() fails to take into account integer overflow case. Signed-off-by: Hiroaki SHIMODA Cc: Tom Herbert Cc: Eric Dumazet Cc: Denys Fedoryshchenko Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- lib/dynamic_queue_limits.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/dynamic_queue_limits.c b/lib/dynamic_queue_limits.c index 6ab4587d052b..c87eb76f2fd4 100644 --- a/lib/dynamic_queue_limits.c +++ b/lib/dynamic_queue_limits.c @@ -10,7 +10,7 @@ #include #include -#define POSDIFF(A, B) ((A) > (B) ? (A) - (B) : 0) +#define POSDIFF(A, B) ((int)((A) - (B)) > 0 ? (A) - (B) : 0) /* Records completed count and recalculates the queue limit */ void dql_completed(struct dql *dql, unsigned int count) -- cgit v1.2.3-58-ga151 From 25426b794efdc70dde7fd3134dc56fac3e7d562d Mon Sep 17 00:00:00 2001 From: Hiroaki SHIMODA Date: Wed, 30 May 2012 12:25:19 +0000 Subject: bql: Avoid unneeded limit decrement. When below pattern is observed, TIME dql_queued() dql_completed() | a) initial state | | b) X bytes queued V c) Y bytes queued d) X bytes completed e) Z bytes queued f) Y bytes completed a) dql->limit has already some value and there is no in-flight packet. b) X bytes queued. c) Y bytes queued and excess limit. d) X bytes completed and dql->prev_ovlimit is set and also dql->prev_num_queued is set Y. e) Z bytes queued. f) Y bytes completed. inprogress and prev_inprogress are true. At f), according to the comment, all_prev_completed becomes true and limit should be increased. But POSDIFF() ignores (completed == dql->prev_num_queued) case, so limit is decreased. Signed-off-by: Hiroaki SHIMODA Cc: Tom Herbert Cc: Eric Dumazet Cc: Denys Fedoryshchenko Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- lib/dynamic_queue_limits.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/dynamic_queue_limits.c b/lib/dynamic_queue_limits.c index c87eb76f2fd4..0fafa77f4036 100644 --- a/lib/dynamic_queue_limits.c +++ b/lib/dynamic_queue_limits.c @@ -11,12 +11,14 @@ #include #define POSDIFF(A, B) ((int)((A) - (B)) > 0 ? (A) - (B) : 0) +#define AFTER_EQ(A, B) ((int)((A) - (B)) >= 0) /* Records completed count and recalculates the queue limit */ void dql_completed(struct dql *dql, unsigned int count) { unsigned int inprogress, prev_inprogress, limit; - unsigned int ovlimit, all_prev_completed, completed; + unsigned int ovlimit, completed; + bool all_prev_completed; /* Can't complete more than what's in queue */ BUG_ON(count > dql->num_queued - dql->num_completed); @@ -26,7 +28,7 @@ void dql_completed(struct dql *dql, unsigned int count) ovlimit = POSDIFF(dql->num_queued - dql->num_completed, limit); inprogress = dql->num_queued - completed; prev_inprogress = dql->prev_num_queued - dql->num_completed; - all_prev_completed = POSDIFF(completed, dql->prev_num_queued); + all_prev_completed = AFTER_EQ(completed, dql->prev_num_queued); if ((ovlimit && !inprogress) || (dql->prev_ovlimit && all_prev_completed)) { -- cgit v1.2.3-58-ga151 From 914bec1011a25f65cdc94988a6f974bfb9a3c10d Mon Sep 17 00:00:00 2001 From: Hiroaki SHIMODA Date: Wed, 30 May 2012 12:25:37 +0000 Subject: bql: Avoid possible inconsistent calculation. dql->num_queued could change while processing dql_completed(). To provide consistent calculation, added an on stack variable. Signed-off-by: Hiroaki SHIMODA Cc: Tom Herbert Cc: Eric Dumazet Cc: Denys Fedoryshchenko Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- lib/dynamic_queue_limits.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/dynamic_queue_limits.c b/lib/dynamic_queue_limits.c index 0fafa77f4036..0777c5a45fa0 100644 --- a/lib/dynamic_queue_limits.c +++ b/lib/dynamic_queue_limits.c @@ -17,16 +17,18 @@ void dql_completed(struct dql *dql, unsigned int count) { unsigned int inprogress, prev_inprogress, limit; - unsigned int ovlimit, completed; + unsigned int ovlimit, completed, num_queued; bool all_prev_completed; + num_queued = ACCESS_ONCE(dql->num_queued); + /* Can't complete more than what's in queue */ - BUG_ON(count > dql->num_queued - dql->num_completed); + BUG_ON(count > num_queued - dql->num_completed); completed = dql->num_completed + count; limit = dql->limit; - ovlimit = POSDIFF(dql->num_queued - dql->num_completed, limit); - inprogress = dql->num_queued - completed; + ovlimit = POSDIFF(num_queued - dql->num_completed, limit); + inprogress = num_queued - completed; prev_inprogress = dql->prev_num_queued - dql->num_completed; all_prev_completed = AFTER_EQ(completed, dql->prev_num_queued); @@ -106,7 +108,7 @@ void dql_completed(struct dql *dql, unsigned int count) dql->prev_ovlimit = ovlimit; dql->prev_last_obj_cnt = dql->last_obj_cnt; dql->num_completed = completed; - dql->prev_num_queued = dql->num_queued; + dql->prev_num_queued = num_queued; } EXPORT_SYMBOL(dql_completed); -- cgit v1.2.3-58-ga151