All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Herrmann <andreas.herrmann@calxeda.com>
To: Ben Hutchings <bhutchings@solarflare.com>
Cc: Rob Herring <rob.herring@calxeda.com>,
	Grant Likely <grant.likely@linaro.org>,
	"David S. Miller" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH] net: calxedaxgmac: Fix panic caused by MTU change of active interface
Date: Wed, 6 Nov 2013 20:25:53 +0100	[thread overview]
Message-ID: <20131106192553.GH5661@alberich> (raw)
In-Reply-To: <1383765756.1520.24.camel@bwh-desktop.uk.level5networks.com>

On Wed, Nov 06, 2013 at 02:22:36PM -0500, Ben Hutchings wrote:
> On Wed, 2013-11-06 at 14:31 +0100, Andreas Herrmann wrote:
> [...]
> > diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
> > index 48f5288..8eb422a 100644
> > --- a/drivers/net/ethernet/calxeda/xgmac.c
> > +++ b/drivers/net/ethernet/calxeda/xgmac.c
> > @@ -1067,6 +1067,10 @@ static int xgmac_stop(struct net_device *dev)
> >  
> >  	writel(0, priv->base + XGMAC_DMA_INTR_ENA);
> >  
> > +	netif_tx_lock_bh(dev);
> > +	netif_stop_queue(dev);
> > +	netif_tx_unlock_bh(dev);
> > +
> [...]
> 
> There is already a call to netif_stop_queue() at the beginning of this
> function, but without locking.  I think it's the wrong place because the
> NAPI poller may still call netif_wake_queue() at that point.  You're
> putting this in the *right* place, but I think you should remove the
> first call.

Ok.

> Also this sequence is also equivalent to netif_tx_disable(), except that
> that also works for multiqueue net devices.

Ok, wasn't aware of this.

I'll send a new patch.


Thanks,

Andreas

  reply	other threads:[~2013-11-06 19:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-06 13:31 [PATCH] net: calxedaxgmac: Fix panic caused by MTU change of active interface Andreas Herrmann
2013-11-06 19:22 ` Ben Hutchings
2013-11-06 19:25   ` Andreas Herrmann [this message]
2013-11-07 11:07 ` [PATCH v2] " Andreas Herrmann

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=20131106192553.GH5661@alberich \
    --to=andreas.herrmann@calxeda.com \
    --cc=bhutchings@solarflare.com \
    --cc=davem@davemloft.net \
    --cc=grant.likely@linaro.org \
    --cc=netdev@vger.kernel.org \
    --cc=rob.herring@calxeda.com \
    /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.