[PATCH] spi: core: Fix deadlock when sending messages

[PATCH] spi: core: Fix deadlock when sending messages

[Jon Hunter]

The function __spi_pump_messages() is called by spi_pump_messages() and
__spi_sync(). The function __spi_sync() has an argument 'bus_locked'
that indicates if it is called with the SPI bus mutex held or not. If
'bus_locked' is false then __spi_sync() will acquire the mutex itself.

Commit 556351f14e74 ("spi: introduce accelerated read support for spi
flash devices") made a change to acquire the SPI bus mutex within
__spi_pump_messages(). However, this change did not check to see if the
mutex is already held. If __spi_sync() is called with the mutex held
(ie. 'bus_locked' is true), then a deadlock occurs when
__spi_pump_messages() is called.

Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
__spi_pump_messages() and only acquire the mutex if not already held. In
the case where __spi_pump_messages() is called from spi_pump_messages()
it is assumed that the mutex is not held and so call
__spi_pump_messages() with 'bus_locked' set to false. Finally, move the
unlocking of the mutex to the end of the __spi_pump_messages() function
to simplify the code and only call cond_resched() if there are no
errors.

Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---

This deadlock is seen on the Tegra124 Nyan Big chromebook and prevents
the board from booting -next.

 drivers/spi/spi.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index fe0196328aa0..e699fec9ddc5 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1062,7 +1062,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * inside spi_sync(); the queue extraction handling at the top of the
  * function should deal with this safely.
  */
-static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
+static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
+				bool bus_locked)
 {
 	unsigned long flags;
 	bool was_busy = false;
@@ -1158,7 +1159,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 		}
 	}
 
-	mutex_lock(&master->bus_lock_mutex);
+	if (!bus_locked)
+		mutex_lock(&master->bus_lock_mutex);
+
 	trace_spi_message_start(master->cur_msg);
 
 	if (master->prepare_message) {
@@ -1168,8 +1171,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 				"failed to prepare message: %d\n", ret);
 			master->cur_msg->status = ret;
 			spi_finalize_current_message(master);
-			mutex_unlock(&master->bus_lock_mutex);
-			return;
+			goto out;
 		}
 		master->cur_msg_prepared = true;
 	}
@@ -1178,21 +1180,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 	if (ret) {
 		master->cur_msg->status = ret;
 		spi_finalize_current_message(master);
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
 
 	ret = master->transfer_one_message(master, master->cur_msg);
 	if (ret) {
 		dev_err(&master->dev,
 			"failed to transfer one message from queue\n");
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
-	mutex_unlock(&master->bus_lock_mutex);
+
+out:
+	if (!bus_locked)
+		mutex_unlock(&master->bus_lock_mutex);
 
 	/* Prod the scheduler in case transfer_one() was busy waiting */
-	cond_resched();
+	if (!ret)
+		cond_resched();
 }
 
 /**
@@ -1204,7 +1208,7 @@ static void spi_pump_messages(struct kthread_work *work)
 	struct spi_master *master =
 		container_of(work, struct spi_master, pump_messages);
 
-	__spi_pump_messages(master, true);
+	__spi_pump_messages(master, true, false);
 }
 
 static int spi_init_queue(struct spi_master *master)
@@ -2814,7 +2818,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
 						       spi_sync_immediate);
 			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
 						       spi_sync_immediate);
-			__spi_pump_messages(master, false);
+			__spi_pump_messages(master, false, bus_locked);
 		}
 
 		wait_for_completion(&done);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] spi: core: Fix deadlock when sending messages

[Jon Hunter]

The function __spi_pump_messages() is called by spi_pump_messages() and
__spi_sync(). The function __spi_sync() has an argument 'bus_locked'
that indicates if it is called with the SPI bus mutex held or not. If
'bus_locked' is false then __spi_sync() will acquire the mutex itself.

Commit 556351f14e74 ("spi: introduce accelerated read support for spi
flash devices") made a change to acquire the SPI bus mutex within
__spi_pump_messages(). However, this change did not check to see if the
mutex is already held. If __spi_sync() is called with the mutex held
(ie. 'bus_locked' is true), then a deadlock occurs when
__spi_pump_messages() is called.

Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
__spi_pump_messages() and only acquire the mutex if not already held. In
the case where __spi_pump_messages() is called from spi_pump_messages()
it is assumed that the mutex is not held and so call
__spi_pump_messages() with 'bus_locked' set to false. Finally, move the
unlocking of the mutex to the end of the __spi_pump_messages() function
to simplify the code and only call cond_resched() if there are no
errors.

Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---

This deadlock is seen on the Tegra124 Nyan Big chromebook and prevents
the board from booting -next.

 drivers/spi/spi.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index fe0196328aa0..e699fec9ddc5 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1062,7 +1062,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * inside spi_sync(); the queue extraction handling at the top of the
  * function should deal with this safely.
  */
-static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
+static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
+				bool bus_locked)
 {
 	unsigned long flags;
 	bool was_busy = false;
@@ -1158,7 +1159,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 		}
 	}
 
-	mutex_lock(&master->bus_lock_mutex);
+	if (!bus_locked)
+		mutex_lock(&master->bus_lock_mutex);
+
 	trace_spi_message_start(master->cur_msg);
 
 	if (master->prepare_message) {
@@ -1168,8 +1171,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 				"failed to prepare message: %d\n", ret);
 			master->cur_msg->status = ret;
 			spi_finalize_current_message(master);
-			mutex_unlock(&master->bus_lock_mutex);
-			return;
+			goto out;
 		}
 		master->cur_msg_prepared = true;
 	}
@@ -1178,21 +1180,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 	if (ret) {
 		master->cur_msg->status = ret;
 		spi_finalize_current_message(master);
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
 
 	ret = master->transfer_one_message(master, master->cur_msg);
 	if (ret) {
 		dev_err(&master->dev,
 			"failed to transfer one message from queue\n");
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
-	mutex_unlock(&master->bus_lock_mutex);
+
+out:
+	if (!bus_locked)
+		mutex_unlock(&master->bus_lock_mutex);
 
 	/* Prod the scheduler in case transfer_one() was busy waiting */
-	cond_resched();
+	if (!ret)
+		cond_resched();
 }
 
 /**
@@ -1204,7 +1208,7 @@ static void spi_pump_messages(struct kthread_work *work)
 	struct spi_master *master =
 		container_of(work, struct spi_master, pump_messages);
 
-	__spi_pump_messages(master, true);
+	__spi_pump_messages(master, true, false);
 }
 
 static int spi_init_queue(struct spi_master *master)
@@ -2814,7 +2818,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
 						       spi_sync_immediate);
 			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
 						       spi_sync_immediate);
-			__spi_pump_messages(master, false);
+			__spi_pump_messages(master, false, bus_locked);
 		}
 
 		wait_for_completion(&done);
-- 
2.1.4

Re: [PATCH] spi: core: Fix deadlock when sending messages

[Jon Hunter]

Adding Vignesh ...

On 08/03/16 09:49, Jon Hunter wrote:
> The function __spi_pump_messages() is called by spi_pump_messages() and
> __spi_sync(). The function __spi_sync() has an argument 'bus_locked'
> that indicates if it is called with the SPI bus mutex held or not. If
> 'bus_locked' is false then __spi_sync() will acquire the mutex itself.
> 
> Commit 556351f14e74 ("spi: introduce accelerated read support for spi
> flash devices") made a change to acquire the SPI bus mutex within
> __spi_pump_messages(). However, this change did not check to see if the
> mutex is already held. If __spi_sync() is called with the mutex held
> (ie. 'bus_locked' is true), then a deadlock occurs when
> __spi_pump_messages() is called.
> 
> Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
> __spi_pump_messages() and only acquire the mutex if not already held. In
> the case where __spi_pump_messages() is called from spi_pump_messages()
> it is assumed that the mutex is not held and so call
> __spi_pump_messages() with 'bus_locked' set to false. Finally, move the
> unlocking of the mutex to the end of the __spi_pump_messages() function
> to simplify the code and only call cond_resched() if there are no
> errors.
> 
> Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")
> 
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> 
> This deadlock is seen on the Tegra124 Nyan Big chromebook and prevents
> the board from booting -next.
> 
>  drivers/spi/spi.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index fe0196328aa0..e699fec9ddc5 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1062,7 +1062,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
>   * inside spi_sync(); the queue extraction handling at the top of the
>   * function should deal with this safely.
>   */
> -static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
> +static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
> +				bool bus_locked)
>  {
>  	unsigned long flags;
>  	bool was_busy = false;
> @@ -1158,7 +1159,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  		}
>  	}
>  
> -	mutex_lock(&master->bus_lock_mutex);
> +	if (!bus_locked)
> +		mutex_lock(&master->bus_lock_mutex);
> +
>  	trace_spi_message_start(master->cur_msg);
>  
>  	if (master->prepare_message) {
> @@ -1168,8 +1171,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  				"failed to prepare message: %d\n", ret);
>  			master->cur_msg->status = ret;
>  			spi_finalize_current_message(master);
> -			mutex_unlock(&master->bus_lock_mutex);
> -			return;
> +			goto out;
>  		}
>  		master->cur_msg_prepared = true;
>  	}
> @@ -1178,21 +1180,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  	if (ret) {
>  		master->cur_msg->status = ret;
>  		spi_finalize_current_message(master);
> -		mutex_unlock(&master->bus_lock_mutex);
> -		return;
> +		goto out;
>  	}
>  
>  	ret = master->transfer_one_message(master, master->cur_msg);
>  	if (ret) {
>  		dev_err(&master->dev,
>  			"failed to transfer one message from queue\n");
> -		mutex_unlock(&master->bus_lock_mutex);
> -		return;
> +		goto out;
>  	}
> -	mutex_unlock(&master->bus_lock_mutex);
> +
> +out:
> +	if (!bus_locked)
> +		mutex_unlock(&master->bus_lock_mutex);
>  
>  	/* Prod the scheduler in case transfer_one() was busy waiting */
> -	cond_resched();
> +	if (!ret)
> +		cond_resched();
>  }
>  
>  /**
> @@ -1204,7 +1208,7 @@ static void spi_pump_messages(struct kthread_work *work)
>  	struct spi_master *master =
>  		container_of(work, struct spi_master, pump_messages);
>  
> -	__spi_pump_messages(master, true);
> +	__spi_pump_messages(master, true, false);
>  }
>  
>  static int spi_init_queue(struct spi_master *master)
> @@ -2814,7 +2818,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
>  						       spi_sync_immediate);
>  			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
>  						       spi_sync_immediate);
> -			__spi_pump_messages(master, false);
> +			__spi_pump_messages(master, false, bus_locked);
>  		}
>  
>  		wait_for_completion(&done);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] spi: core: Fix deadlock when sending messages

[Jon Hunter]

Adding Vignesh ...

On 08/03/16 09:49, Jon Hunter wrote:
> The function __spi_pump_messages() is called by spi_pump_messages() and
> __spi_sync(). The function __spi_sync() has an argument 'bus_locked'
> that indicates if it is called with the SPI bus mutex held or not. If
> 'bus_locked' is false then __spi_sync() will acquire the mutex itself.
> 
> Commit 556351f14e74 ("spi: introduce accelerated read support for spi
> flash devices") made a change to acquire the SPI bus mutex within
> __spi_pump_messages(). However, this change did not check to see if the
> mutex is already held. If __spi_sync() is called with the mutex held
> (ie. 'bus_locked' is true), then a deadlock occurs when
> __spi_pump_messages() is called.
> 
> Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
> __spi_pump_messages() and only acquire the mutex if not already held. In
> the case where __spi_pump_messages() is called from spi_pump_messages()
> it is assumed that the mutex is not held and so call
> __spi_pump_messages() with 'bus_locked' set to false. Finally, move the
> unlocking of the mutex to the end of the __spi_pump_messages() function
> to simplify the code and only call cond_resched() if there are no
> errors.
> 
> Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> 
> This deadlock is seen on the Tegra124 Nyan Big chromebook and prevents
> the board from booting -next.
> 
>  drivers/spi/spi.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index fe0196328aa0..e699fec9ddc5 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1062,7 +1062,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
>   * inside spi_sync(); the queue extraction handling at the top of the
>   * function should deal with this safely.
>   */
> -static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
> +static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
> +				bool bus_locked)
>  {
>  	unsigned long flags;
>  	bool was_busy = false;
> @@ -1158,7 +1159,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  		}
>  	}
>  
> -	mutex_lock(&master->bus_lock_mutex);
> +	if (!bus_locked)
> +		mutex_lock(&master->bus_lock_mutex);
> +
>  	trace_spi_message_start(master->cur_msg);
>  
>  	if (master->prepare_message) {
> @@ -1168,8 +1171,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  				"failed to prepare message: %d\n", ret);
>  			master->cur_msg->status = ret;
>  			spi_finalize_current_message(master);
> -			mutex_unlock(&master->bus_lock_mutex);
> -			return;
> +			goto out;
>  		}
>  		master->cur_msg_prepared = true;
>  	}
> @@ -1178,21 +1180,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  	if (ret) {
>  		master->cur_msg->status = ret;
>  		spi_finalize_current_message(master);
> -		mutex_unlock(&master->bus_lock_mutex);
> -		return;
> +		goto out;
>  	}
>  
>  	ret = master->transfer_one_message(master, master->cur_msg);
>  	if (ret) {
>  		dev_err(&master->dev,
>  			"failed to transfer one message from queue\n");
> -		mutex_unlock(&master->bus_lock_mutex);
> -		return;
> +		goto out;
>  	}
> -	mutex_unlock(&master->bus_lock_mutex);
> +
> +out:
> +	if (!bus_locked)
> +		mutex_unlock(&master->bus_lock_mutex);
>  
>  	/* Prod the scheduler in case transfer_one() was busy waiting */
> -	cond_resched();
> +	if (!ret)
> +		cond_resched();
>  }
>  
>  /**
> @@ -1204,7 +1208,7 @@ static void spi_pump_messages(struct kthread_work *work)
>  	struct spi_master *master =
>  		container_of(work, struct spi_master, pump_messages);
>  
> -	__spi_pump_messages(master, true);
> +	__spi_pump_messages(master, true, false);
>  }
>  
>  static int spi_init_queue(struct spi_master *master)
> @@ -2814,7 +2818,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
>  						       spi_sync_immediate);
>  			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
>  						       spi_sync_immediate);
> -			__spi_pump_messages(master, false);
> +			__spi_pump_messages(master, false, bus_locked);
>  		}
>  
>  		wait_for_completion(&done);
> 

Re: [PATCH] spi: core: Fix deadlock when sending messages

[Jon Hunter]

Adding Vignesh ...

On 08/03/16 09:49, Jon Hunter wrote:
> The function __spi_pump_messages() is called by spi_pump_messages() and
> __spi_sync(). The function __spi_sync() has an argument 'bus_locked'
> that indicates if it is called with the SPI bus mutex held or not. If
> 'bus_locked' is false then __spi_sync() will acquire the mutex itself.
> 
> Commit 556351f14e74 ("spi: introduce accelerated read support for spi
> flash devices") made a change to acquire the SPI bus mutex within
> __spi_pump_messages(). However, this change did not check to see if the
> mutex is already held. If __spi_sync() is called with the mutex held
> (ie. 'bus_locked' is true), then a deadlock occurs when
> __spi_pump_messages() is called.
> 
> Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
> __spi_pump_messages() and only acquire the mutex if not already held. In
> the case where __spi_pump_messages() is called from spi_pump_messages()
> it is assumed that the mutex is not held and so call
> __spi_pump_messages() with 'bus_locked' set to false. Finally, move the
> unlocking of the mutex to the end of the __spi_pump_messages() function
> to simplify the code and only call cond_resched() if there are no
> errors.
> 
> Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")
> 
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> 
> This deadlock is seen on the Tegra124 Nyan Big chromebook and prevents
> the board from booting -next.
> 
>  drivers/spi/spi.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index fe0196328aa0..e699fec9ddc5 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1062,7 +1062,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
>   * inside spi_sync(); the queue extraction handling at the top of the
>   * function should deal with this safely.
>   */
> -static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
> +static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
> +				bool bus_locked)
>  {
>  	unsigned long flags;
>  	bool was_busy = false;
> @@ -1158,7 +1159,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  		}
>  	}
>  
> -	mutex_lock(&master->bus_lock_mutex);
> +	if (!bus_locked)
> +		mutex_lock(&master->bus_lock_mutex);
> +
>  	trace_spi_message_start(master->cur_msg);
>  
>  	if (master->prepare_message) {
> @@ -1168,8 +1171,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  				"failed to prepare message: %d\n", ret);
>  			master->cur_msg->status = ret;
>  			spi_finalize_current_message(master);
> -			mutex_unlock(&master->bus_lock_mutex);
> -			return;
> +			goto out;
>  		}
>  		master->cur_msg_prepared = true;
>  	}
> @@ -1178,21 +1180,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
>  	if (ret) {
>  		master->cur_msg->status = ret;
>  		spi_finalize_current_message(master);
> -		mutex_unlock(&master->bus_lock_mutex);
> -		return;
> +		goto out;
>  	}
>  
>  	ret = master->transfer_one_message(master, master->cur_msg);
>  	if (ret) {
>  		dev_err(&master->dev,
>  			"failed to transfer one message from queue\n");
> -		mutex_unlock(&master->bus_lock_mutex);
> -		return;
> +		goto out;
>  	}
> -	mutex_unlock(&master->bus_lock_mutex);
> +
> +out:
> +	if (!bus_locked)
> +		mutex_unlock(&master->bus_lock_mutex);
>  
>  	/* Prod the scheduler in case transfer_one() was busy waiting */
> -	cond_resched();
> +	if (!ret)
> +		cond_resched();
>  }
>  
>  /**
> @@ -1204,7 +1208,7 @@ static void spi_pump_messages(struct kthread_work *work)
>  	struct spi_master *master =
>  		container_of(work, struct spi_master, pump_messages);
>  
> -	__spi_pump_messages(master, true);
> +	__spi_pump_messages(master, true, false);
>  }
>  
>  static int spi_init_queue(struct spi_master *master)
> @@ -2814,7 +2818,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
>  						       spi_sync_immediate);
>  			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
>  						       spi_sync_immediate);
> -			__spi_pump_messages(master, false);
> +			__spi_pump_messages(master, false, bus_locked);
>  		}
>  
>  		wait_for_completion(&done);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] spi: core: Fix deadlock when sending messages

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3620 bytes --]

Hi Jon,

[auto build test WARNING on spi/for-next]
[also build test WARNING on next-20160308]
[cannot apply to v4.5-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jon-Hunter/spi-core-Fix-deadlock-when-sending-messages/20160308-175230
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi for-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/init.h:1: warning: no structured comments found
   kernel/sys.c:1: warning: no structured comments found
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   drivers/dma-buf/reservation.c:1: warning: no structured comments found
   include/linux/reservation.h:1: warning: no structured comments found
   include/linux/spi/spi.h:933: error: Cannot parse struct or union!
>> drivers/spi/spi.c:1067: warning: No description found for parameter 'bus_locked'
   drivers/spi/spi.c:2358: warning: No description found for parameter 'gfp'
>> drivers/spi/spi.c:1067: warning: No description found for parameter 'bus_locked'
   drivers/spi/spi.c:2358: warning: No description found for parameter 'gfp'

vim +/bus_locked +1067 drivers/spi/spi.c

b158935f7 Mark Brown    2013-10-05  1051  
ffbbdd213 Linus Walleij 2012-02-22  1052  /**
fc9e0f71f Mark Brown    2014-12-10  1053   * __spi_pump_messages - function which processes spi message queue
fc9e0f71f Mark Brown    2014-12-10  1054   * @master: master to process queue for
fc9e0f71f Mark Brown    2014-12-10  1055   * @in_kthread: true if we are in the context of the message pump thread
ffbbdd213 Linus Walleij 2012-02-22  1056   *
ffbbdd213 Linus Walleij 2012-02-22  1057   * This function checks if there is any spi message in the queue that
ffbbdd213 Linus Walleij 2012-02-22  1058   * needs processing and if so call out to the driver to initialize hardware
ffbbdd213 Linus Walleij 2012-02-22  1059   * and transfer each message.
ffbbdd213 Linus Walleij 2012-02-22  1060   *
0461a4149 Mark Brown    2014-12-09  1061   * Note that it is called both from the kthread itself and also from
0461a4149 Mark Brown    2014-12-09  1062   * inside spi_sync(); the queue extraction handling at the top of the
0461a4149 Mark Brown    2014-12-09  1063   * function should deal with this safely.
ffbbdd213 Linus Walleij 2012-02-22  1064   */
1f9706090 Jon Hunter    2016-03-08  1065  static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
1f9706090 Jon Hunter    2016-03-08  1066  				bool bus_locked)
ffbbdd213 Linus Walleij 2012-02-22 @1067  {
ffbbdd213 Linus Walleij 2012-02-22  1068  	unsigned long flags;
ffbbdd213 Linus Walleij 2012-02-22  1069  	bool was_busy = false;
ffbbdd213 Linus Walleij 2012-02-22  1070  	int ret;
ffbbdd213 Linus Walleij 2012-02-22  1071  
983aee5d7 Mark Brown    2014-12-09  1072  	/* Lock queue */
ffbbdd213 Linus Walleij 2012-02-22  1073  	spin_lock_irqsave(&master->queue_lock, flags);
983aee5d7 Mark Brown    2014-12-09  1074  
983aee5d7 Mark Brown    2014-12-09  1075  	/* Make sure we are not already running a message */

:::::: The code at line 1067 was first introduced by commit
:::::: ffbbdd21329f3e15eeca6df2d4bc11c04d9d91c0 spi: create a message queueing infrastructure

:::::: TO: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
:::::: CC: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6229 bytes --]

Re: [PATCH] spi: core: Fix deadlock when sending messages

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3564 bytes --]

Hi Jon,

[auto build test WARNING on spi/for-next]
[also build test WARNING on next-20160308]
[cannot apply to v4.5-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jon-Hunter/spi-core-Fix-deadlock-when-sending-messages/20160308-175230
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi for-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/init.h:1: warning: no structured comments found
   kernel/sys.c:1: warning: no structured comments found
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   drivers/dma-buf/reservation.c:1: warning: no structured comments found
   include/linux/reservation.h:1: warning: no structured comments found
   include/linux/spi/spi.h:933: error: Cannot parse struct or union!
>> drivers/spi/spi.c:1067: warning: No description found for parameter 'bus_locked'
   drivers/spi/spi.c:2358: warning: No description found for parameter 'gfp'
>> drivers/spi/spi.c:1067: warning: No description found for parameter 'bus_locked'
   drivers/spi/spi.c:2358: warning: No description found for parameter 'gfp'

vim +/bus_locked +1067 drivers/spi/spi.c

b158935f7 Mark Brown    2013-10-05  1051  
ffbbdd213 Linus Walleij 2012-02-22  1052  /**
fc9e0f71f Mark Brown    2014-12-10  1053   * __spi_pump_messages - function which processes spi message queue
fc9e0f71f Mark Brown    2014-12-10  1054   * @master: master to process queue for
fc9e0f71f Mark Brown    2014-12-10  1055   * @in_kthread: true if we are in the context of the message pump thread
ffbbdd213 Linus Walleij 2012-02-22  1056   *
ffbbdd213 Linus Walleij 2012-02-22  1057   * This function checks if there is any spi message in the queue that
ffbbdd213 Linus Walleij 2012-02-22  1058   * needs processing and if so call out to the driver to initialize hardware
ffbbdd213 Linus Walleij 2012-02-22  1059   * and transfer each message.
ffbbdd213 Linus Walleij 2012-02-22  1060   *
0461a4149 Mark Brown    2014-12-09  1061   * Note that it is called both from the kthread itself and also from
0461a4149 Mark Brown    2014-12-09  1062   * inside spi_sync(); the queue extraction handling at the top of the
0461a4149 Mark Brown    2014-12-09  1063   * function should deal with this safely.
ffbbdd213 Linus Walleij 2012-02-22  1064   */
1f9706090 Jon Hunter    2016-03-08  1065  static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
1f9706090 Jon Hunter    2016-03-08  1066  				bool bus_locked)
ffbbdd213 Linus Walleij 2012-02-22 @1067  {
ffbbdd213 Linus Walleij 2012-02-22  1068  	unsigned long flags;
ffbbdd213 Linus Walleij 2012-02-22  1069  	bool was_busy = false;
ffbbdd213 Linus Walleij 2012-02-22  1070  	int ret;
ffbbdd213 Linus Walleij 2012-02-22  1071  
983aee5d7 Mark Brown    2014-12-09  1072  	/* Lock queue */
ffbbdd213 Linus Walleij 2012-02-22  1073  	spin_lock_irqsave(&master->queue_lock, flags);
983aee5d7 Mark Brown    2014-12-09  1074  
983aee5d7 Mark Brown    2014-12-09  1075  	/* Make sure we are not already running a message */

:::::: The code at line 1067 was first introduced by commit
:::::: ffbbdd21329f3e15eeca6df2d4bc11c04d9d91c0 spi: create a message queueing infrastructure

:::::: TO: Linus Walleij <linus.walleij@linaro.org>
:::::: CC: Grant Likely <grant.likely@secretlab.ca>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6229 bytes --]

Re: [PATCH] spi: core: Fix deadlock when sending messages

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3620 bytes --]

Hi Jon,

[auto build test WARNING on spi/for-next]
[also build test WARNING on next-20160308]
[cannot apply to v4.5-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jon-Hunter/spi-core-Fix-deadlock-when-sending-messages/20160308-175230
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi for-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/init.h:1: warning: no structured comments found
   kernel/sys.c:1: warning: no structured comments found
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   drivers/dma-buf/reservation.c:1: warning: no structured comments found
   include/linux/reservation.h:1: warning: no structured comments found
   include/linux/spi/spi.h:933: error: Cannot parse struct or union!
>> drivers/spi/spi.c:1067: warning: No description found for parameter 'bus_locked'
   drivers/spi/spi.c:2358: warning: No description found for parameter 'gfp'
>> drivers/spi/spi.c:1067: warning: No description found for parameter 'bus_locked'
   drivers/spi/spi.c:2358: warning: No description found for parameter 'gfp'

vim +/bus_locked +1067 drivers/spi/spi.c

b158935f7 Mark Brown    2013-10-05  1051  
ffbbdd213 Linus Walleij 2012-02-22  1052  /**
fc9e0f71f Mark Brown    2014-12-10  1053   * __spi_pump_messages - function which processes spi message queue
fc9e0f71f Mark Brown    2014-12-10  1054   * @master: master to process queue for
fc9e0f71f Mark Brown    2014-12-10  1055   * @in_kthread: true if we are in the context of the message pump thread
ffbbdd213 Linus Walleij 2012-02-22  1056   *
ffbbdd213 Linus Walleij 2012-02-22  1057   * This function checks if there is any spi message in the queue that
ffbbdd213 Linus Walleij 2012-02-22  1058   * needs processing and if so call out to the driver to initialize hardware
ffbbdd213 Linus Walleij 2012-02-22  1059   * and transfer each message.
ffbbdd213 Linus Walleij 2012-02-22  1060   *
0461a4149 Mark Brown    2014-12-09  1061   * Note that it is called both from the kthread itself and also from
0461a4149 Mark Brown    2014-12-09  1062   * inside spi_sync(); the queue extraction handling at the top of the
0461a4149 Mark Brown    2014-12-09  1063   * function should deal with this safely.
ffbbdd213 Linus Walleij 2012-02-22  1064   */
1f9706090 Jon Hunter    2016-03-08  1065  static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
1f9706090 Jon Hunter    2016-03-08  1066  				bool bus_locked)
ffbbdd213 Linus Walleij 2012-02-22 @1067  {
ffbbdd213 Linus Walleij 2012-02-22  1068  	unsigned long flags;
ffbbdd213 Linus Walleij 2012-02-22  1069  	bool was_busy = false;
ffbbdd213 Linus Walleij 2012-02-22  1070  	int ret;
ffbbdd213 Linus Walleij 2012-02-22  1071  
983aee5d7 Mark Brown    2014-12-09  1072  	/* Lock queue */
ffbbdd213 Linus Walleij 2012-02-22  1073  	spin_lock_irqsave(&master->queue_lock, flags);
983aee5d7 Mark Brown    2014-12-09  1074  
983aee5d7 Mark Brown    2014-12-09  1075  	/* Make sure we are not already running a message */

:::::: The code at line 1067 was first introduced by commit
:::::: ffbbdd21329f3e15eeca6df2d4bc11c04d9d91c0 spi: create a message queueing infrastructure

:::::: TO: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
:::::: CC: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 6229 bytes --]

[PATCH V2] spi: core: Fix deadlock when sending messages

[Jon Hunter]

The function __spi_pump_messages() is called by spi_pump_messages() and
__spi_sync(). The function __spi_sync() has an argument 'bus_locked'
that indicates if it is called with the SPI bus mutex held or not. If
'bus_locked' is false then __spi_sync() will acquire the mutex itself.

Commit 556351f14e74 ("spi: introduce accelerated read support for spi
flash devices") made a change to acquire the SPI bus mutex within
__spi_pump_messages(). However, this change did not check to see if the
mutex is already held. If __spi_sync() is called with the mutex held
(ie. 'bus_locked' is true), then a deadlock occurs when
__spi_pump_messages() is called.

Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
__spi_pump_messages() and only acquire the mutex if not already held. In
the case where __spi_pump_messages() is called from spi_pump_messages()
it is assumed that the mutex is not held and so call
__spi_pump_messages() with 'bus_locked' set to false. Finally, move the
unlocking of the mutex to the end of the __spi_pump_messages() function
to simplify the code and only call cond_resched() if there are no
errors.

Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---

Changes v1-v2:
- Updated kernel-doc for new parameter added to __spi_pump_messages()

 drivers/spi/spi.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index fe0196328aa0..78b7e899ce76 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1053,6 +1053,7 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * __spi_pump_messages - function which processes spi message queue
  * @master: master to process queue for
  * @in_kthread: true if we are in the context of the message pump thread
+ * @bus_locked: true if the bus mutex is held when calling this function
  *
  * This function checks if there is any spi message in the queue that
  * needs processing and if so call out to the driver to initialize hardware
@@ -1062,7 +1063,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * inside spi_sync(); the queue extraction handling at the top of the
  * function should deal with this safely.
  */
-static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
+static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
+				bool bus_locked)
 {
 	unsigned long flags;
 	bool was_busy = false;
@@ -1158,7 +1160,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 		}
 	}
 
-	mutex_lock(&master->bus_lock_mutex);
+	if (!bus_locked)
+		mutex_lock(&master->bus_lock_mutex);
+
 	trace_spi_message_start(master->cur_msg);
 
 	if (master->prepare_message) {
@@ -1168,8 +1172,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 				"failed to prepare message: %d\n", ret);
 			master->cur_msg->status = ret;
 			spi_finalize_current_message(master);
-			mutex_unlock(&master->bus_lock_mutex);
-			return;
+			goto out;
 		}
 		master->cur_msg_prepared = true;
 	}
@@ -1178,21 +1181,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 	if (ret) {
 		master->cur_msg->status = ret;
 		spi_finalize_current_message(master);
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
 
 	ret = master->transfer_one_message(master, master->cur_msg);
 	if (ret) {
 		dev_err(&master->dev,
 			"failed to transfer one message from queue\n");
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
-	mutex_unlock(&master->bus_lock_mutex);
+
+out:
+	if (!bus_locked)
+		mutex_unlock(&master->bus_lock_mutex);
 
 	/* Prod the scheduler in case transfer_one() was busy waiting */
-	cond_resched();
+	if (!ret)
+		cond_resched();
 }
 
 /**
@@ -1204,7 +1209,7 @@ static void spi_pump_messages(struct kthread_work *work)
 	struct spi_master *master =
 		container_of(work, struct spi_master, pump_messages);
 
-	__spi_pump_messages(master, true);
+	__spi_pump_messages(master, true, false);
 }
 
 static int spi_init_queue(struct spi_master *master)
@@ -2814,7 +2819,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
 						       spi_sync_immediate);
 			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
 						       spi_sync_immediate);
-			__spi_pump_messages(master, false);
+			__spi_pump_messages(master, false, bus_locked);
 		}
 
 		wait_for_completion(&done);
-- 
2.1.4

[PATCH V2] spi: core: Fix deadlock when sending messages

[Jon Hunter]

The function __spi_pump_messages() is called by spi_pump_messages() and
__spi_sync(). The function __spi_sync() has an argument 'bus_locked'
that indicates if it is called with the SPI bus mutex held or not. If
'bus_locked' is false then __spi_sync() will acquire the mutex itself.

Commit 556351f14e74 ("spi: introduce accelerated read support for spi
flash devices") made a change to acquire the SPI bus mutex within
__spi_pump_messages(). However, this change did not check to see if the
mutex is already held. If __spi_sync() is called with the mutex held
(ie. 'bus_locked' is true), then a deadlock occurs when
__spi_pump_messages() is called.

Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
__spi_pump_messages() and only acquire the mutex if not already held. In
the case where __spi_pump_messages() is called from spi_pump_messages()
it is assumed that the mutex is not held and so call
__spi_pump_messages() with 'bus_locked' set to false. Finally, move the
unlocking of the mutex to the end of the __spi_pump_messages() function
to simplify the code and only call cond_resched() if there are no
errors.

Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---

Changes v1-v2:
- Updated kernel-doc for new parameter added to __spi_pump_messages()

 drivers/spi/spi.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index fe0196328aa0..78b7e899ce76 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1053,6 +1053,7 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * __spi_pump_messages - function which processes spi message queue
  * @master: master to process queue for
  * @in_kthread: true if we are in the context of the message pump thread
+ * @bus_locked: true if the bus mutex is held when calling this function
  *
  * This function checks if there is any spi message in the queue that
  * needs processing and if so call out to the driver to initialize hardware
@@ -1062,7 +1063,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * inside spi_sync(); the queue extraction handling at the top of the
  * function should deal with this safely.
  */
-static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
+static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
+				bool bus_locked)
 {
 	unsigned long flags;
 	bool was_busy = false;
@@ -1158,7 +1160,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 		}
 	}
 
-	mutex_lock(&master->bus_lock_mutex);
+	if (!bus_locked)
+		mutex_lock(&master->bus_lock_mutex);
+
 	trace_spi_message_start(master->cur_msg);
 
 	if (master->prepare_message) {
@@ -1168,8 +1172,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 				"failed to prepare message: %d\n", ret);
 			master->cur_msg->status = ret;
 			spi_finalize_current_message(master);
-			mutex_unlock(&master->bus_lock_mutex);
-			return;
+			goto out;
 		}
 		master->cur_msg_prepared = true;
 	}
@@ -1178,21 +1181,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 	if (ret) {
 		master->cur_msg->status = ret;
 		spi_finalize_current_message(master);
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
 
 	ret = master->transfer_one_message(master, master->cur_msg);
 	if (ret) {
 		dev_err(&master->dev,
 			"failed to transfer one message from queue\n");
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
-	mutex_unlock(&master->bus_lock_mutex);
+
+out:
+	if (!bus_locked)
+		mutex_unlock(&master->bus_lock_mutex);
 
 	/* Prod the scheduler in case transfer_one() was busy waiting */
-	cond_resched();
+	if (!ret)
+		cond_resched();
 }
 
 /**
@@ -1204,7 +1209,7 @@ static void spi_pump_messages(struct kthread_work *work)
 	struct spi_master *master =
 		container_of(work, struct spi_master, pump_messages);
 
-	__spi_pump_messages(master, true);
+	__spi_pump_messages(master, true, false);
 }
 
 static int spi_init_queue(struct spi_master *master)
@@ -2814,7 +2819,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
 						       spi_sync_immediate);
 			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
 						       spi_sync_immediate);
-			__spi_pump_messages(master, false);
+			__spi_pump_messages(master, false, bus_locked);
 		}
 
 		wait_for_completion(&done);
-- 
2.1.4

[PATCH V2] spi: core: Fix deadlock when sending messages

[Jon Hunter]

The function __spi_pump_messages() is called by spi_pump_messages() and
__spi_sync(). The function __spi_sync() has an argument 'bus_locked'
that indicates if it is called with the SPI bus mutex held or not. If
'bus_locked' is false then __spi_sync() will acquire the mutex itself.

Commit 556351f14e74 ("spi: introduce accelerated read support for spi
flash devices") made a change to acquire the SPI bus mutex within
__spi_pump_messages(). However, this change did not check to see if the
mutex is already held. If __spi_sync() is called with the mutex held
(ie. 'bus_locked' is true), then a deadlock occurs when
__spi_pump_messages() is called.

Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
__spi_pump_messages() and only acquire the mutex if not already held. In
the case where __spi_pump_messages() is called from spi_pump_messages()
it is assumed that the mutex is not held and so call
__spi_pump_messages() with 'bus_locked' set to false. Finally, move the
unlocking of the mutex to the end of the __spi_pump_messages() function
to simplify the code and only call cond_resched() if there are no
errors.

Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---

Changes v1-v2:
- Updated kernel-doc for new parameter added to __spi_pump_messages()

 drivers/spi/spi.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index fe0196328aa0..78b7e899ce76 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1053,6 +1053,7 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * __spi_pump_messages - function which processes spi message queue
  * @master: master to process queue for
  * @in_kthread: true if we are in the context of the message pump thread
+ * @bus_locked: true if the bus mutex is held when calling this function
  *
  * This function checks if there is any spi message in the queue that
  * needs processing and if so call out to the driver to initialize hardware
@@ -1062,7 +1063,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * inside spi_sync(); the queue extraction handling at the top of the
  * function should deal with this safely.
  */
-static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
+static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
+				bool bus_locked)
 {
 	unsigned long flags;
 	bool was_busy = false;
@@ -1158,7 +1160,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 		}
 	}
 
-	mutex_lock(&master->bus_lock_mutex);
+	if (!bus_locked)
+		mutex_lock(&master->bus_lock_mutex);
+
 	trace_spi_message_start(master->cur_msg);
 
 	if (master->prepare_message) {
@@ -1168,8 +1172,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 				"failed to prepare message: %d\n", ret);
 			master->cur_msg->status = ret;
 			spi_finalize_current_message(master);
-			mutex_unlock(&master->bus_lock_mutex);
-			return;
+			goto out;
 		}
 		master->cur_msg_prepared = true;
 	}
@@ -1178,21 +1181,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 	if (ret) {
 		master->cur_msg->status = ret;
 		spi_finalize_current_message(master);
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
 
 	ret = master->transfer_one_message(master, master->cur_msg);
 	if (ret) {
 		dev_err(&master->dev,
 			"failed to transfer one message from queue\n");
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
-	mutex_unlock(&master->bus_lock_mutex);
+
+out:
+	if (!bus_locked)
+		mutex_unlock(&master->bus_lock_mutex);
 
 	/* Prod the scheduler in case transfer_one() was busy waiting */
-	cond_resched();
+	if (!ret)
+		cond_resched();
 }
 
 /**
@@ -1204,7 +1209,7 @@ static void spi_pump_messages(struct kthread_work *work)
 	struct spi_master *master =
 		container_of(work, struct spi_master, pump_messages);
 
-	__spi_pump_messages(master, true);
+	__spi_pump_messages(master, true, false);
 }
 
 static int spi_init_queue(struct spi_master *master)
@@ -2814,7 +2819,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
 						       spi_sync_immediate);
 			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
 						       spi_sync_immediate);
-			__spi_pump_messages(master, false);
+			__spi_pump_messages(master, false, bus_locked);
 		}
 
 		wait_for_completion(&done);
-- 
2.1.4

Applied "spi: core: Fix deadlock when sending messages" to the spi tree

[Mark Brown]

The patch

   spi: core: Fix deadlock when sending messages

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 49023d2e4ead0fc9e0896331037746b267d46ad4 Mon Sep 17 00:00:00 2001
From: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Date: Tue, 8 Mar 2016 12:28:20 +0000
Subject: [PATCH] spi: core: Fix deadlock when sending messages

The function __spi_pump_messages() is called by spi_pump_messages() and
__spi_sync(). The function __spi_sync() has an argument 'bus_locked'
that indicates if it is called with the SPI bus mutex held or not. If
'bus_locked' is false then __spi_sync() will acquire the mutex itself.

Commit 556351f14e74 ("spi: introduce accelerated read support for spi
flash devices") made a change to acquire the SPI bus mutex within
__spi_pump_messages(). However, this change did not check to see if the
mutex is already held. If __spi_sync() is called with the mutex held
(ie. 'bus_locked' is true), then a deadlock occurs when
__spi_pump_messages() is called.

Fix this deadlock by passing the 'bus_locked' state from __spi_sync() to
__spi_pump_messages() and only acquire the mutex if not already held. In
the case where __spi_pump_messages() is called from spi_pump_messages()
it is assumed that the mutex is not held and so call
__spi_pump_messages() with 'bus_locked' set to false. Finally, move the
unlocking of the mutex to the end of the __spi_pump_messages() function
to simplify the code and only call cond_resched() if there are no
errors.

Fixes: 556351f14e74 ("spi: introduce accelerated read support for spi flash devices")

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 0b2bbf144460..f565cc8901a6 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1047,6 +1047,7 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * __spi_pump_messages - function which processes spi message queue
  * @master: master to process queue for
  * @in_kthread: true if we are in the context of the message pump thread
+ * @bus_locked: true if the bus mutex is held when calling this function
  *
  * This function checks if there is any spi message in the queue that
  * needs processing and if so call out to the driver to initialize hardware
@@ -1056,7 +1057,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
  * inside spi_sync(); the queue extraction handling at the top of the
  * function should deal with this safely.
  */
-static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
+static void __spi_pump_messages(struct spi_master *master, bool in_kthread,
+				bool bus_locked)
 {
 	unsigned long flags;
 	bool was_busy = false;
@@ -1152,7 +1154,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 		}
 	}
 
-	mutex_lock(&master->bus_lock_mutex);
+	if (!bus_locked)
+		mutex_lock(&master->bus_lock_mutex);
+
 	trace_spi_message_start(master->cur_msg);
 
 	if (master->prepare_message) {
@@ -1162,8 +1166,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 				"failed to prepare message: %d\n", ret);
 			master->cur_msg->status = ret;
 			spi_finalize_current_message(master);
-			mutex_unlock(&master->bus_lock_mutex);
-			return;
+			goto out;
 		}
 		master->cur_msg_prepared = true;
 	}
@@ -1172,21 +1175,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
 	if (ret) {
 		master->cur_msg->status = ret;
 		spi_finalize_current_message(master);
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
 
 	ret = master->transfer_one_message(master, master->cur_msg);
 	if (ret) {
 		dev_err(&master->dev,
 			"failed to transfer one message from queue\n");
-		mutex_unlock(&master->bus_lock_mutex);
-		return;
+		goto out;
 	}
-	mutex_unlock(&master->bus_lock_mutex);
+
+out:
+	if (!bus_locked)
+		mutex_unlock(&master->bus_lock_mutex);
 
 	/* Prod the scheduler in case transfer_one() was busy waiting */
-	cond_resched();
+	if (!ret)
+		cond_resched();
 }
 
 /**
@@ -1198,7 +1203,7 @@ static void spi_pump_messages(struct kthread_work *work)
 	struct spi_master *master =
 		container_of(work, struct spi_master, pump_messages);
 
-	__spi_pump_messages(master, true);
+	__spi_pump_messages(master, true, false);
 }
 
 static int spi_init_queue(struct spi_master *master)
@@ -2462,7 +2467,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message,
 						       spi_sync_immediate);
 			SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
 						       spi_sync_immediate);
-			__spi_pump_messages(master, false);
+			__spi_pump_messages(master, false, bus_locked);
 		}
 
 		wait_for_completion(&done);
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html