All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michał Nazarewicz" <m.nazarewicz@samsung.com>
To: David Miller <davem@davemloft.net>,
	Eric Dumazet <eric.dumazet@gmail.com>
Cc: dbrownell@users.sourceforge.net, gregkh@suse.de,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH] usb: gadget: rndis: don't use dev_get_stats()
Date: Mon, 20 Sep 2010 21:00:35 +0200	[thread overview]
Message-ID: <op.vjchq9lx7p4s8u@pikus> (raw)
In-Reply-To: <1285008527.2323.52.camel@edumazet-laptop>

>> From: Michal Nazarewicz <m.nazarewicz@samsung.com>
>>> This commit removes the call to dev_get_stats() from the
>>> gen_ndis_query_resp() function.  Since spin_lock_bh() was
>>> added to dev_txq_stats_fold() the call started causing
>>> warnings.  This is because gen_ndis_query_resp() can be
>>> (indirectly) called from rndis_command_complete() which is
>>> called with interrupts disabled.

> Le lundi 20 septembre 2010 à 11:27 -0700, David Miller a écrit :
>> The way this works is dev_get_stats() takes that "temp" object the
>> caller provides, it writes the correct statistics into it (with any
>> necessary translations), and then passes back a pointer to it.
[...]
>> Like I originally suggested, you need to rearrange the code in this
>> driver such that the gen_ndis_query_resp() work happens in a tasklet,
>> workqueue, or some other non-hardware-irq context.

On Mon, 20 Sep 2010 20:48:47 +0200, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Hmm, maybe its a bit difficult to fix this problem for stable kernel.
>
> What we could do is assume rndis wont use a device driver that actually
> needs txq tx stats folding, and just use following interim patch ?
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b9b22a3..31d5424 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5288,6 +5288,14 @@ void dev_txq_stats_fold(const struct net_device *dev,
>  	unsigned int i;
>  	struct netdev_queue *txq;
>+	/* temporary hack : rndis calls us under hard irq */
> +	if (WARN_ON_ONCE(in_irq())) {
> +		stats->tx_bytes   = dev->stats.tx_bytes;
> +		stats->tx_packets = dev->stats.tx_packets;
> +		stats->tx_dropped = dev->stats.tx_dropped;
> +		return;
> +	}
> +
>  	for (i = 0; i < dev->num_tx_queues; i++) {
>  		txq = netdev_get_tx_queue(dev, i);
>  		spin_lock_bh(&txq->_xmit_lock);

I think it's better to put such temporary fix inside RNDIS code.  It
seems there is no need to clutter the whole subsystem with temporary
fix.

-- 
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Michał "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--

  reply	other threads:[~2010-09-20 18:58 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-20 18:23 [PATCH] usb: gadget: rndis: don't use dev_get_stats() Michal Nazarewicz
2010-09-20 18:27 ` David Miller
2010-09-20 18:48   ` Eric Dumazet
2010-09-20 19:00     ` Michał Nazarewicz [this message]
2010-09-20 18:58   ` Michał Nazarewicz

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=op.vjchq9lx7p4s8u@pikus \
    --to=m.nazarewicz@samsung.com \
    --cc=davem@davemloft.net \
    --cc=dbrownell@users.sourceforge.net \
    --cc=eric.dumazet@gmail.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@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.