Hi Jamal, J Hadi Salim <[EMAIL PROTECTED]> wrote on 05/09/2007 06:26:43 PM:
> On Wed, 2007-09-05 at 01:12 -0700, David Miller wrote: > > > Something this evening is obviously making it impossible > > for my brain to understand this function and your patch, > > so I'm going to sleep on it and try again tomorrow :-) > > It is one of those areas that are hard to size-up in a blink;-> > Gut-feeling: It doesnt sit right with me as well. > > With (2.6.18-rxX++) QDISC_RUNNING changes that mean only one of N CPUs > will be dequeueing while the N-1 maybe enqueueing concurently. All N Concurrently is not possible, since everyone needs the queue_lock to add/delete. Did I misunderstand your comment ? > CPUs contend for the queue lock; and theres a possible window between > releasing the queue lock by the dequeuer-CPU and enqueuer-CPU for a > race. The dequeuer-CPU entering one last time helps. The dev->queue_lock is held by both enqueue'r and dequeue'r (though the dequeue'r drops it before calling xmit). But once the dequeue'r re-gets the lock, it is guaranteed that no one else has the lock Other CPU's trying to add will block on the lock, or if they have already added by getting the lock for a short time while my CPU was doing the xmit, then their qdisc_run returns doing nothing as RUNNING is true. Since I am holding a lock in these two changed areas till I return back to __qdisc_run (which clears the RUNNING bit) and then drop the lock, there is no way packets can be on the queue while I falsely return 0, or no packets on the queue while I falsely return -1. I hope my explanation was not confusing. > Krishna, you probably saw this "wasted entry into qdisc" under low > traffic conditions with more than likely only one CPU sending, am i > correct? Under heavier traffic when we have multiple CPUs funneling to Actually, I have never seen this problem, just code reading while trying to implement something else in this area, for which I plan to bug Dave in a few days. Thanks, - KK > the same device, that entry is not really a "waste" because we endup > only go in once per X number of packets enqueued on the qdisc and that > check is absolutely necessary because a different CPU may have enqueued > while you were not looking. In the case of low traffic, X=1 - so it is a > waste there albeit a necessary one. > > cheers, > jamal > > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
