On Wed, Nov 16, 2016 at 1:01 PM, Brian Starkey <brian.star...@arm.com> wrote: > On Wed, Nov 16, 2016 at 10:49:06AM -0800, Eric Dumazet wrote: >> >> On Wed, Nov 16, 2016 at 10:01 AM, Brian Starkey <brian.star...@arm.com> >> wrote: >> >>> >>> The smc91x driver does seem to have some trickiness around softirqs. >>> I'm not familiar with net drivers, but I'll see if I can figure >>> anything out there. >> >> >> Oh this code looks ugly :( >> >> Do you have CONFIG_SMP=y or not ? > > > Yeah CONFIG_SMP=y (and CONFIG_PREEMPT=y too, fwiw). > > I did try forcing it into the no-op locking (as though config SMP > wasn't set), it didn't help (and it doesn't look like that would be > safe with CONFIG_PREEMPT=y either). > > The bit in smc_hardware_send_pkt looks like skipping softirq > invocation when there's already one running wouldn't give the same > behaviour as before: > > if (!smc_special_trylock(&lp->lock, flags)) { > netif_stop_queue(dev); > tasklet_schedule(&lp->tx_task); > return; > } > > ... that said, I've no idea if that matters. > > Of course I also don't know if the network driver is even to blame :-( >
I believe the problem is in SMC_WAIT_MMU_BUSY() Could you try this patch ? (inlined and attached) diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c index 65077c77082a2f042117a0889c2b15099c58eae5..4ef653ae8564c6d2a5120cdaef12db1e1b218b39 100644 --- a/drivers/net/ethernet/smsc/smc91x.c +++ b/drivers/net/ethernet/smsc/smc91x.c @@ -229,19 +229,22 @@ static inline void PRINT_PKT(u_char *buf, int length) { } * if at all, but let's avoid deadlocking the system if the hardware * decides to go south. */ -#define SMC_WAIT_MMU_BUSY(lp) do { \ - if (unlikely(SMC_GET_MMU_CMD(lp) & MC_BUSY)) { \ - unsigned long timeout = jiffies + 2; \ - while (SMC_GET_MMU_CMD(lp) & MC_BUSY) { \ - if (time_after(jiffies, timeout)) { \ - netdev_dbg(dev, "timeout %s line %d\n", \ - __FILE__, __LINE__); \ - break; \ - } \ - cpu_relax(); \ - } \ - } \ -} while (0) +static void SMC_WAIT_MMU_BUSY(struct smc_local *lp) +{ + unsigned long timeout = jiffies + 2; + unsigned int count = 10000; + + while (SMC_GET_MMU_CMD(lp) & MC_BUSY) { + count--; + if (!count || time_after(jiffies, timeout)) { + netdev_dbg(lp->dev, "timeout %s line %d\n", + __FILE__, __LINE__); + break; + } + /* TODO : investigate using cond_resched() from allowed contexts */ + cpu_relax(); + } +} /*
diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c index 65077c77082a2f042117a0889c2b15099c58eae5..4ef653ae8564c6d2a5120cdaef12db1e1b218b39 100644 --- a/drivers/net/ethernet/smsc/smc91x.c +++ b/drivers/net/ethernet/smsc/smc91x.c @@ -229,19 +229,22 @@ static inline void PRINT_PKT(u_char *buf, int length) { } * if at all, but let's avoid deadlocking the system if the hardware * decides to go south. */ -#define SMC_WAIT_MMU_BUSY(lp) do { \ - if (unlikely(SMC_GET_MMU_CMD(lp) & MC_BUSY)) { \ - unsigned long timeout = jiffies + 2; \ - while (SMC_GET_MMU_CMD(lp) & MC_BUSY) { \ - if (time_after(jiffies, timeout)) { \ - netdev_dbg(dev, "timeout %s line %d\n", \ - __FILE__, __LINE__); \ - break; \ - } \ - cpu_relax(); \ - } \ - } \ -} while (0) +static void SMC_WAIT_MMU_BUSY(struct smc_local *lp) +{ + unsigned long timeout = jiffies + 2; + unsigned int count = 10000; + + while (SMC_GET_MMU_CMD(lp) & MC_BUSY) { + count--; + if (!count || time_after(jiffies, timeout)) { + netdev_dbg(lp->dev, "timeout %s line %d\n", + __FILE__, __LINE__); + break; + } + /* TODO : investigate using cond_resched() from allowed contexts */ + cpu_relax(); + } +} /*