[patch] iwlwifi: turn on a lockdep assertion

[patch] iwlwifi: turn on a lockdep assertion

[Dan Carpenter]

CMD_SYNC is zero so the if (cmd->flags & CMD_SYNC) is never true.  I
think it should be if (cmd->flags == CMD_SYNC).  This enables a lockdep
assertion.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index e55ec6c..c5ccdfa 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -1259,7 +1259,7 @@ int iwl_dvm_send_cmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
 	 * the mutex, this ensures we don't try to send two
 	 * (or more) synchronous commands at a time.
 	 */
-	if (cmd->flags & CMD_SYNC)
+	if (cmd->flags == CMD_SYNC)
 		lockdep_assert_held(&priv->mutex);
 
 	if (priv->ucode_owner == IWL_OWNERSHIP_TM &&

Re: [patch] iwlwifi: turn on a lockdep assertion

[Johannes Berg]

On Sat, 2012-06-09 at 18:57 +0300, Dan Carpenter wrote:
> CMD_SYNC is zero so the if (cmd->flags & CMD_SYNC) is never true.  I
> think it should be if (cmd->flags == CMD_SYNC).  This enables a lockdep
> assertion.

Good catch, thanks. It should really be if (!(cmd->flags & CMD_ASYNC))
though I think as there could be other flags set which don't make it
non-sync.

I'll commit a fix on Monday unless you want to send a separate patch
(which I'll then pick up on Monday :-) )

johannes

Re: [patch] iwlwifi: turn on a lockdep assertion

[Dan Carpenter]

On Sat, Jun 09, 2012 at 08:12:56PM +0200, Johannes Berg wrote:
> On Sat, 2012-06-09 at 18:57 +0300, Dan Carpenter wrote:
> > CMD_SYNC is zero so the if (cmd->flags & CMD_SYNC) is never true.  I
> > think it should be if (cmd->flags == CMD_SYNC).  This enables a lockdep
> > assertion.
> 
> Good catch, thanks. It should really be if (!(cmd->flags & CMD_ASYNC))
> though I think as there could be other flags set which don't make it
> non-sync.
> 
> I'll commit a fix on Monday unless you want to send a separate patch
> (which I'll then pick up on Monday :-) )


Sure.  I'll resend it.

regards,
dan carpenter

[patch v2] iwlwifi: turn on a lockdep assertion

[Dan Carpenter]

CMD_SYNC is zero so the if (cmd->flags & CMD_SYNC) is never true and we
never check the assertion.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: enables the assertion even more than v1 (based on feedback from
Johannes Berg).

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index e55ec6c..5c04895 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -1259,7 +1259,7 @@ int iwl_dvm_send_cmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
 	 * the mutex, this ensures we don't try to send two
 	 * (or more) synchronous commands at a time.
 	 */
-	if (cmd->flags & CMD_SYNC)
+	if (!(cmd->flags & CMD_ASYNC))
 		lockdep_assert_held(&priv->mutex);
 
 	if (priv->ucode_owner == IWL_OWNERSHIP_TM &&

Re: [patch v2] iwlwifi: turn on a lockdep assertion

[walter harms]

Am 10.06.2012 13:25, schrieb Dan Carpenter:
> CMD_SYNC is zero so the if (cmd->flags & CMD_SYNC) is never true and we
> never check the assertion.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: enables the assertion even more than v1 (based on feedback from
> Johannes Berg).

The important part is CMD_SYNC -> CMD_ASYNC

re,
 wh


> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> index e55ec6c..5c04895 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> @@ -1259,7 +1259,7 @@ int iwl_dvm_send_cmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
>  	 * the mutex, this ensures we don't try to send two
>  	 * (or more) synchronous commands at a time.
>  	 */
> -	if (cmd->flags & CMD_SYNC)
> +	if (!(cmd->flags & CMD_ASYNC))
>  		lockdep_assert_held(&priv->mutex);
>  
>  	if (priv->ucode_owner == IWL_OWNERSHIP_TM &&
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [patch v2] iwlwifi: turn on a lockdep assertion

[Dan Carpenter]

On Sun, Jun 10, 2012 at 05:25:05PM +0200, walter harms wrote:
> 
> 
> Am 10.06.2012 13:25, schrieb Dan Carpenter:
> > CMD_SYNC is zero so the if (cmd->flags & CMD_SYNC) is never true and we
> > never check the assertion.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: enables the assertion even more than v1 (based on feedback from
> > Johannes Berg).
> 
> The important part is CMD_SYNC -> CMD_ASYNC

Not really, no.  I considered Johannes's version for my first patch
but I decided to be conservative because I didn't know the code well
enough to say what is safe.

regards,
dan carpenter

Re: [patch v2] iwlwifi: turn on a lockdep assertion

[Johannes Berg]

On Sun, 2012-06-10 at 14:25 +0300, Dan Carpenter wrote:
> CMD_SYNC is zero so the if (cmd->flags & CMD_SYNC) is never true and we
> never check the assertion.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: enables the assertion even more than v1 (based on feedback from
> Johannes Berg).

Thanks, I picked this up.

johannes