Thread (4 messages) 4 messages, 3 authors, 2014-09-02

Re: question about drivers/net/ethernet/ti/cpsw.c

From: Daniel Mack <zonque@gmail.com>
Date: 2014-09-02 09:34:19

On 09/02/2014 03:11 AM, David Miller wrote:
From: Julia Lawall <redacted>
Date: Thu, 28 Aug 2014 21:26:55 +0200 (CEST)
quoted
I wonder if the following patch:

commit aa1a15e2d9199711cdcc9399fdb22544ab835a83
Author: Daniel Mack [off-list ref]
Date:   Sat Sep 21 00:50:38 2013 +0530

introduced a race condition in drivers/net/ethernet/ti/cpsw.c.  I was 
looking at an old version of the file (Linux 3.10), and it has

 clean_irq_ret:
         for (i = 0; i < priv->num_irqs; i++)
                 free_irq(priv->irqs_table[i], priv);

at the beginning of the cleanup code of the probe function (cpsw_probe).  
The above patch replaces request_irq by devm_request_irq and gets rid of 
the above cleanup code.  But that moves the stopping of the interrupts 
after the following code at the end of the function:

free_netdev(priv->ndev);

The interrupt handler (cpsw_interrupt) does reference priv->ndev:

	if (netif_running(priv->ndev)) {
                napi_schedule(&priv->napi);
                return IRQ_HANDLED;
        }

so perhaps this could be a problem.  The same happens in the remove 
function.
It could definitely be a problem.

Probably it would be better for this device to request IRQs in open
and release them in close like so many other networking drivers do.
Thanks for spotting this, Julia!
I'll be working on a fix for this as soon as I can.


Best regards,
Daniel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help