From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Subject: The proper way of delaying tx in a driver
Date: Wed, 23 Jul 2008 16:37:23 +1000 [thread overview]
Message-ID: <1216795043.11027.311.camel@pasglop> (raw)
Hi !
(Dave: this is basically the conversation we had on IRC, I think at that
point it's worth discussing here and I'll see if I can update the
documentation along fix fixing a handful of drivers).
So the problem: various drivers need to temporarily stop TX, ie, make
sure their hard_hard_xmit() is not running and will not be called for a
certain amount of time, in order to perform various housekeeping tasks.
This ranges from things like change_mtu() to reset tasks, or whatever
other things driver may want to do that require that locking.
For a short amount of time, just locking the tx lock
(netif_tx_lock{_bh}) does the job just fine. So let's ignore that. We
are in the case of a driver that wants to do something long, such as
reallocating the entire RX ring (change_mtu) or resetting hardware, and
potentially want to sleep / schedule.
The drivers historically use netif_stop_queue()/netif_wake_queue() to do
that. This is fishy due to locking, but let's assume that at this stage
we have a clueful driver writer, and thus like tg3, we do
netif_tx_disable / netif_wake_queue instead.
The above unfortunately hits the new WARN_ON() as Dave pointed out, it's
not legal to call netif_wake_queue() before a driver's open() function
called netif_start_queue().
Drivers like tg3 seem to be at least -somewhat- careful, and only do
those things when netif_running(). However, unless I missed something,
this is true from just before the driver open() is called, that is, too
early for closing the race.
So the question is, what is the proper approach ?
I'm happy to help fixing tg3, sungem and emac at least as I'm somewhat
familiar with those 3 drivers once we decide what is the right sequence
of operations here.
Cheers,
Ben.
next reply other threads:[~2008-07-23 6:37 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-23 6:37 Benjamin Herrenschmidt [this message]
2008-07-23 8:11 ` The proper way of delaying tx in a driver David Miller
2008-07-23 8:19 ` Benjamin Herrenschmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1216795043.11027.311.camel@pasglop \
--to=benh@kernel.crashing.org \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.