On 05/30/2012 07:49 AM, Eric Dumazet wrote:
> On Wed, 2012-05-30 at 20:29 +0900, Hiroaki SHIMODA wrote:
>> On Wed, 30 May 2012 13:08:27 +0200
>> Eric Dumazet<eric.duma...@gmail.com> wrote:
>>
>>> On Wed, 2012-05-30 at 19:43 +0900, Hiroaki SHIMODA wrote:
>>>
>>>> While examining ping problem, below pattern is often 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), if I read the comment correctly, all_prev_completed becomes
>>>> true and limit should be increased. But POSDIFF() ignores
>>>> (A == B) case, so limit is decreased.
>>> Which POSDIFF(), because there are many ;)
>> I mean,
>> all_prev_completed = POSDIFF(completed, dql->prev_num_queued);
>>
>>> By the way, given complexity of this I suggest you split your ideas in
>>> independent patches.
>> In this case, here is the patch what I thinking.
>>
>> diff --git a/lib/dynamic_queue_limits.c b/lib/dynamic_queue_limits.c
>> @@ -11,12 +11,14 @@
>> #include<linux/dynamic_queue_limits.h>
>>
>> #define POSDIFF(A, B) ((A)> (B) ? (A) - (B) : 0)
>> +#define #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)) {
> I am fine with this one.
>
> Can you send official patches please ?
While this looks encouraging, BQL presently overbuffers by about a
factor of 2 in low (sub 100Mbit) scenarios
on the hardware I have available to me.
I look forward to re-running benchmarks with this patch however.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit
http://communities.intel.com/community/wired
