| Message ID | 4D512A27.2030406@dsn.okisemi.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com> Date: Tue, 08 Feb 2011 20:33:59 +0900 > @@ -531,12 +533,8 @@ void pch_gbe_reinit_locked(struct pch_gbe_adapter *adapter) > { > struct net_device *netdev = adapter->netdev; > > - rtnl_lock(); > - if (netif_running(netdev)) { > - pch_gbe_down(adapter); > - pch_gbe_up(adapter); > - } > - rtnl_unlock(); > + pch_gbe_down(adapter); > + pch_gbe_up(adapter); Are you sure you can just blindly delete the netif_running() check here? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Miller <davem@davemloft.net> Date: Tue, 08 Feb 2011 16:40:30 -0800 (PST) >> @@ -531,12 +533,8 @@ void pch_gbe_reinit_locked(struct pch_gbe_adapter >> *adapter) >> { >> struct net_device *netdev = adapter->netdev; >> >> - rtnl_lock(); >> - if (netif_running(netdev)) { >> - pch_gbe_down(adapter); >> - pch_gbe_up(adapter); >> - } >> - rtnl_unlock(); >> + pch_gbe_down(adapter); >> + pch_gbe_up(adapter); > >Are you sure you can just blindly delete the netif_running() check here? Yes, sure. pch_gbe_reinit_locked() is called after confirming of netif_running() except for pch_gbe_reset_task() function. This netif_running() was redundant. Best regards Toshiharu Okada(OKI semiconductor) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Toshiharu Okada" <toshiharu-linux@dsn.okisemi.com> Date: Wed, 9 Feb 2011 11:04:11 +0900 > From: David Miller <davem@davemloft.net> > Date: Tue, 08 Feb 2011 16:40:30 -0800 (PST) > >>> @@ -531,12 +533,8 @@ void pch_gbe_reinit_locked(struct pch_gbe_adapter >>> *adapter) >>> { >>> struct net_device *netdev = adapter->netdev; >>> >>> - rtnl_lock(); >>> - if (netif_running(netdev)) { >>> - pch_gbe_down(adapter); >>> - pch_gbe_up(adapter); >>> - } >>> - rtnl_unlock(); >>> + pch_gbe_down(adapter); >>> + pch_gbe_up(adapter); >> >>Are you sure you can just blindly delete the netif_running() check here? > > Yes, sure. > pch_gbe_reinit_locked() is called after confirming of netif_running() except > for pch_gbe_reset_task() function. > This netif_running() was redundant. Thanks for explaining, applied, thank you. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: David Miller <davem@davemloft.net> Date: Tue, 08 Feb 2011 22:57:02 -0800 (PST) > From: "Toshiharu Okada" <toshiharu-linux@dsn.okisemi.com> > Date: Wed, 9 Feb 2011 11:04:11 +0900 > >> From: David Miller <davem@davemloft.net> >> Date: Tue, 08 Feb 2011 16:40:30 -0800 (PST) >> >>>> @@ -531,12 +533,8 @@ void pch_gbe_reinit_locked(struct pch_gbe_adapter >>>> *adapter) >>>> { >>>> struct net_device *netdev = adapter->netdev; >>>> >>>> - rtnl_lock(); >>>> - if (netif_running(netdev)) { >>>> - pch_gbe_down(adapter); >>>> - pch_gbe_up(adapter); >>>> - } >>>> - rtnl_unlock(); >>>> + pch_gbe_down(adapter); >>>> + pch_gbe_up(adapter); >>> >>>Are you sure you can just blindly delete the netif_running() check here? >> >> Yes, sure. >> pch_gbe_reinit_locked() is called after confirming of netif_running() except >> for pch_gbe_reset_task() function. >> This netif_running() was redundant. > > Thanks for explaining, applied, thank you. Actually, I had to revert, this patch introduces an obvious compiler warning: drivers/net/pch_gbe/pch_gbe_main.c: In function 'pch_gbe_reinit_locked': drivers/net/pch_gbe/pch_gbe_main.c:533:21: warning: unused variable 'netdev' Sloppy build issues like this reflects very poorly upon the patch submitter. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/pch_gbe/pch_gbe_main.c b/drivers/net/pch_gbe/pch_gbe_main.c index 3248313..0e6922f 100644 --- a/drivers/net/pch_gbe/pch_gbe_main.c +++ b/drivers/net/pch_gbe/pch_gbe_main.c @@ -520,7 +520,9 @@ static void pch_gbe_reset_task(struct work_struct *work) struct pch_gbe_adapter *adapter; adapter = container_of(work, struct pch_gbe_adapter, reset_task); + rtnl_lock(); pch_gbe_reinit_locked(adapter); + rtnl_unlock(); } /** @@ -531,12 +533,8 @@ void pch_gbe_reinit_locked(struct pch_gbe_adapter *adapter) { struct net_device *netdev = adapter->netdev; - rtnl_lock(); - if (netif_running(netdev)) { - pch_gbe_down(adapter); - pch_gbe_up(adapter); - } - rtnl_unlock(); + pch_gbe_down(adapter); + pch_gbe_up(adapter); } /**
This driver will be in a deadlock, When the rx offload is set by ethtool. So, The pch_gbe_reinit_locked function was modified. Signed-off-by: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com> --- drivers/net/pch_gbe/pch_gbe_main.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-)