netdev - Re: [PATCH take 2] pkt_sched: Fix qdisc_watchdog() vs. dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 11 Sep 2008 03:49:55 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	herbert@...dor.apana.org.au
Cc:	jarkao2@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH take 2] pkt_sched: Fix qdisc_watchdog() vs.
 dev_deactivate() race

From: Herbert Xu <herbert@...dor.apana.org.au>
Date: Thu, 11 Sep 2008 20:45:31 +1000

> On Thu, Sep 11, 2008 at 03:39:56AM -0700, David Miller wrote:
> >
> > I got to looking into this and we do need the qdisc->dev_queue member,
> > see qdisc_run().  So it's not like we can get rid of it if we replace
> > it with ->netdevdev and add a ->root_qdisc backpointer as well.
> 
> That can't be right.  Let's I've got a single qdisc shared by
> n queues.  It makes no sense for qdisc_run to decide to whether
> it should process the qdisc depending on the status of a single
> one out of the n queues.

Well some kind of check has to be there.

I _did_ remove it during my initial implementation, and that
turned into a reported performance regression.

See:

commit 83f36f3f35f4f83fa346bfff58a5deabc78370e5
Author: David S. Miller <davem@...emloft.net>
Date:   Wed Aug 13 02:13:34 2008 -0700

    pkt_sched: Add queue stopped test back to qdisc_run().
    
    Based upon a bug report by Andrew Gallatin on netdev
    with subject "CPU utilization increased in 2.6.27rc"
    
    In commit 37437bb2e1ae8af470dfcd5b4ff454110894ccaf
    ("pkt_sched: Schedule qdiscs instead of netdev_queue.")
    the test of the queue being stopped was erroneously
    removed from qdisc_run().
    
    When the TX queue of the device fills up, this omission
    causes lots of extraneous useless work to be queued up
    to softirq context, where we'll just return immediately
    because the device is still stuffed up.
    
    Signed-off-by: David S. Miller <davem@...emloft.net>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html