diff mbox

[net,RFC,1/2] netdevice: add might_sleep() call to napi_disable

Message ID 20130918201949.8697.50700.stgit@jekeller-desk1.amr.corp.intel.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Jacob Keller Sept. 18, 2013, 8:20 p.m. UTC
napi_disable potentially calls msleep(1) if the NAPI_STATE_SCHED bit is
previously set by something else. Because it does not always call msleep, it
was difficult to track down a bug related to calling napi_disable within
local_bh_disable()d context. This patch adds a might_sleep() call to the
napi_disable routine in order to aid in the future debugging of similar issues.
This will cause a BUG in drivers which have implemented busy polling in a
similar fashion to ixgbe, and which call napi_disable inside the
local_bh_disable()d section where the vector napi lock is taken.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Cc: Eliezer Tamir <eliezer.tamir@linux.intel.com>
Cc: Alex Duyck <alexander.h.duyck@intel.com>
Cc: Hyong-Youb Kim <hykim@myri.com>
Cc: Amir Vadai <amirv@mellanox.com>
Cc: Dmitry Kravkov <dmitry@broadcom.com>
---
 include/linux/netdevice.h |    2 ++
 1 file changed, 2 insertions(+)


--
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

Comments

Amir Vadai Oct. 8, 2013, 12:39 p.m. UTC | #1
On 18/09/2013 23:20, Jacob Keller wrote:
> napi_disable potentially calls msleep(1) if the NAPI_STATE_SCHED bit is
> previously set by something else. Because it does not always call msleep, it
> was difficult to track down a bug related to calling napi_disable within
> local_bh_disable()d context. This patch adds a might_sleep() call to the
> napi_disable routine in order to aid in the future debugging of similar issues.
> This will cause a BUG in drivers which have implemented busy polling in a
> similar fashion to ixgbe, and which call napi_disable inside the
> local_bh_disable()d section where the vector napi lock is taken.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Cc: Eliezer Tamir <eliezer.tamir@linux.intel.com>
> Cc: Alex Duyck <alexander.h.duyck@intel.com>
> Cc: Hyong-Youb Kim <hykim@myri.com>
> Cc: Amir Vadai <amirv@mellanox.com>
> Cc: Dmitry Kravkov <dmitry@broadcom.com>
> ---
>  include/linux/netdevice.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 3de49ac..81dab00 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -483,6 +483,8 @@ extern void napi_hash_del(struct napi_struct *napi);
>   */
>  static inline void napi_disable(struct napi_struct *n)
>  {
> +	might_sleep();
> +
>  	set_bit(NAPI_STATE_DISABLE, &n->state);
>  	while (test_and_set_bit(NAPI_STATE_SCHED, &n->state))
>  		msleep(1);
> 

Works ok with mlx4_en driver.

Acked-By: Amir Vadai <amirv@mellanox.com>
--
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 mbox

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3de49ac..81dab00 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -483,6 +483,8 @@  extern void napi_hash_del(struct napi_struct *napi);
  */
 static inline void napi_disable(struct napi_struct *n)
 {
+	might_sleep();
+
 	set_bit(NAPI_STATE_DISABLE, &n->state);
 	while (test_and_set_bit(NAPI_STATE_SCHED, &n->state))
 		msleep(1);