[lkml]   [2004]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: Lockup with 2.6.9-ac15 related to netconsole
    Francois Romieu wrote:
    > Patrick McHardy <> :
    > [...]
    >>at least the queued messages ordered. But you need to grab
    >>dev->queue_lock, otherwise you risk corrupting qdisc internal data.
    >>You should probably also deal with the noqueue-qdisc, which doesn't
    >>have an enqueue function. So it should look something like this:
    > If I am not mistaken, a failure on spin_trylock + the test on
    > xmit_lock_owner imply that it is safe to directly handle the
    > queue. It means that qdisc_run() has been interrupted on the
    > current cpu and the other paths seem fine as well. Counter-example
    > is welcome (no joke).

    enqueue is only protected by dev->queue_lock, and dev->queue_lock
    is dropped as soon as dev->xmit_lock is grabbed, so any other CPU
    might call enqueue at the same time.


    CPU1 CPU2

    dev_queue_xmit dev_queue_xmit
    lock(dev->queue_lock) lock(dev->queue_lock)
    trylock(dev->xmit_lock), ok
    trylock(dev->xmit_lock), fails
    q->enqueue q->enqueue

    > Of course the patch is completely ugly and violates any layering
    > principle one could think of. It was not submitted for inclusion :o)

    Sure, but I think we should have a short-term workaround until
    a better solution has been invented. Maybe dropping the packets
    would be best for now, it only affects printks issued in paths
    starting at qdisc_restart (-> hard_start_xmit -> ...). Queueing
    the packets might also cause reordering since not all packets
    are queued.

    >>while (!spin_trylock(&np->dev->xmit_lock)) {
    >> if (np->dev->xmit_lock_owner == smp_processor_id()) {
    >> struct Qdisc *q;
    >> rcu_read_lock();
    >> q = rcu_dereference(dev->qdisc);
    >> if (q->enqueue) {
    >> spin_lock(&dev->queue_lock);
    > I'd expect it to deadlock if dev_queue_xmit -> qdisc_run is interrupted
    > on the current cpu and a printk is issued as dev->queue_lock will have
    > been taken elsewhere.

    Hmm this is complicated, let me think some more about it.

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 14:08    [W:0.028 / U:7.800 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site