Netdev List
 help / color / mirror / Atom feed
From: Shreyas Bhatewara <sbhatewara@vmware.com>
To: David Miller <davem@davemloft.net>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"pv-drivers@vmware.com" <pv-drivers@vmware.com>,
	Ronghua Zhang <ronghua@vmware.com>,
	Matthieu Bucchianeri <matthieu@vmware.com>
Subject: Re: [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [4/5] Do not reset when the device is not opened
Date: Thu, 15 Jul 2010 18:20:52 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LRH.2.00.1007151448260.25472@localhost.localdomain> (raw)
In-Reply-To: <20100714.140718.115934967.davem@davemloft.net>



On Wed, 14 Jul 2010, David Miller wrote:

> From: Shreyas Bhatewara <sbhatewara@vmware.com>
> Date: Tue, 13 Jul 2010 17:49:52 -0700 (PDT)
> 
> > 
> > Do not reset when the device is not opened
> > 
> > If a reset is scheduled, and the device goes thru close and open, it
> > may happen that reset and open may run in parallel.
> > The reset code now bails out if the device is not opened.
> >  
> > Signed-off-by: Ronghua Zang <ronghua@vmware.com>
> > Signed-off-by: Matthieu Bucchianeri <matthieu@vmware.com>
> > Signed-off-by: Shreyas Bhatewara <sbhatewara@vmware.com>
> 
> Testing IFF_UP is just making your race hole a little smaller but
> it isn't fixing the problem.
> 
> In fact, there really isn't any legitimate reason for a driver
> to test the IFF_UP state bit.  netif_running() is the correct
> test, and asynchronous work queue things like resets should
> synchronize with open/close using the RTNL lock so that your
> test of netif_running() is a stable one.
> 

Is this what you suggest :

---

Hold rtnl_lock to get the right link state.

While asynchronously resetting the device, hold rtnl_lock to get the 
right value from netif_running. If a reset is scheduled, and the device
goes thru close and open, it may happen that reset and open may run in 
parallel. Holding rtnl_lock will avoid this.

---

 drivers/net/vmxnet3/vmxnet3_drv.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 1b0ce8c..c4d7e42 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2420,6 +2420,7 @@ vmxnet3_reset_work(struct work_struct *data)
 		return;
 
 	/* if the device is closed, we must leave it alone */
+	rtnl_lock();
 	if (netif_running(adapter->netdev)) {
 		printk(KERN_INFO "%s: resetting\n", adapter->netdev->name);
 		vmxnet3_quiesce_dev(adapter);
@@ -2428,6 +2429,7 @@ vmxnet3_reset_work(struct work_struct *data)
 	} else {
 		printk(KERN_INFO "%s: already closed\n", adapter->netdev->name);
 	}
+	rtnl_unlock();
 
 	clear_bit(VMXNET3_STATE_BIT_RESETTING, &adapter->state);
 }

  reply	other threads:[~2010-07-16  1:20 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <alpine.LRH.2.00.1007070125440.4652@localhost.localdomain>
2010-07-07  9:24 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [2/5] Interrupt control bitmap Shreyas Bhatewara
2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
2010-07-14 21:04     ` David Miller
2010-07-16  1:20       ` Shreyas Bhatewara
2010-07-16  5:19         ` David Miller
2010-07-07  9:28 ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
2010-07-07  9:31   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [4/5] Do not reset when the device is not opened Shreyas Bhatewara
2010-07-14  0:49     ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
2010-07-14 21:07       ` David Miller
2010-07-16  1:20         ` Shreyas Bhatewara [this message]
2010-07-16  1:32           ` David Miller
2010-07-16  8:17             ` Shreyas Bhatewara
2010-07-17 23:35               ` David Miller
2010-07-19 17:02                 ` Shreyas Bhatewara
2010-07-19 20:16                   ` David Miller
2010-07-07  9:34   ` [PATCH 2.6.35-rc1] net: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration Shreyas Bhatewara
2010-07-14  0:51     ` [PATCH 2.6.35-rc1] net-next: " Shreyas Bhatewara
2010-07-14 21:11       ` David Miller
2010-07-16  1:21         ` Shreyas Bhatewara
2010-07-16  1:28           ` [PATCH 2.6.35-rc1] net-next: fix LRO feature update in vmxnet3 Shreyas Bhatewara
2010-07-16  5:17             ` David Miller
2010-07-19 20:16           ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [5/5] Respect the interrupt type in VM configuration David Miller
2010-07-14  0:48   ` [PATCH 2.6.35-rc1] net-next: vmxnet3 fixes [3/5] Initialize link state at probe time Shreyas Bhatewara
2010-07-14 21:10     ` David Miller
2010-07-16  1:20       ` Shreyas Bhatewara
2010-07-16  5:44         ` David Miller
2010-07-16  7:51           ` Shreyas Bhatewara
2010-07-18 21:49             ` David Miller

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=alpine.LRH.2.00.1007151448260.25472@localhost.localdomain \
    --to=sbhatewara@vmware.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthieu@vmware.com \
    --cc=netdev@vger.kernel.org \
    --cc=pv-drivers@vmware.com \
    --cc=ronghua@vmware.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox