diff options
| author | Jon Hunter <jonathanh@nvidia.com> | 2016-03-08 07:28:20 -0500 |
|---|---|---|
| committer | Mark Brown <broonie@kernel.org> | 2016-03-08 21:31:39 -0500 |
| commit | 49023d2e4ead0fc9e0896331037746b267d46ad4 (patch) | |
| tree | 4550038559a27f6698a6702b3ee20c833a669f7c | |
| parent | 628269704f19fcfc765499b7158effccfc79b6cf (diff) | |
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@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
| -rw-r--r-- | drivers/spi/spi.c | 29 |
1 files 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); | |||
| 1047 | * __spi_pump_messages - function which processes spi message queue | 1047 | * __spi_pump_messages - function which processes spi message queue |
| 1048 | * @master: master to process queue for | 1048 | * @master: master to process queue for |
| 1049 | * @in_kthread: true if we are in the context of the message pump thread | 1049 | * @in_kthread: true if we are in the context of the message pump thread |
| 1050 | * @bus_locked: true if the bus mutex is held when calling this function | ||
| 1050 | * | 1051 | * |
| 1051 | * This function checks if there is any spi message in the queue that | 1052 | * This function checks if there is any spi message in the queue that |
| 1052 | * needs processing and if so call out to the driver to initialize hardware | 1053 | * needs processing and if so call out to the driver to initialize hardware |
| @@ -1056,7 +1057,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer); | |||
| 1056 | * inside spi_sync(); the queue extraction handling at the top of the | 1057 | * inside spi_sync(); the queue extraction handling at the top of the |
| 1057 | * function should deal with this safely. | 1058 | * function should deal with this safely. |
| 1058 | */ | 1059 | */ |
| 1059 | static void __spi_pump_messages(struct spi_master *master, bool in_kthread) | 1060 | static void __spi_pump_messages(struct spi_master *master, bool in_kthread, |
| 1061 | bool bus_locked) | ||
| 1060 | { | 1062 | { |
| 1061 | unsigned long flags; | 1063 | unsigned long flags; |
| 1062 | bool was_busy = false; | 1064 | bool was_busy = false; |
| @@ -1152,7 +1154,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread) | |||
| 1152 | } | 1154 | } |
| 1153 | } | 1155 | } |
| 1154 | 1156 | ||
| 1155 | mutex_lock(&master->bus_lock_mutex); | 1157 | if (!bus_locked) |
| 1158 | mutex_lock(&master->bus_lock_mutex); | ||
| 1159 | |||
| 1156 | trace_spi_message_start(master->cur_msg); | 1160 | trace_spi_message_start(master->cur_msg); |
| 1157 | 1161 | ||
| 1158 | if (master->prepare_message) { | 1162 | if (master->prepare_message) { |
| @@ -1162,8 +1166,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread) | |||
| 1162 | "failed to prepare message: %d\n", ret); | 1166 | "failed to prepare message: %d\n", ret); |
| 1163 | master->cur_msg->status = ret; | 1167 | master->cur_msg->status = ret; |
| 1164 | spi_finalize_current_message(master); | 1168 | spi_finalize_current_message(master); |
| 1165 | mutex_unlock(&master->bus_lock_mutex); | 1169 | goto out; |
| 1166 | return; | ||
| 1167 | } | 1170 | } |
| 1168 | master->cur_msg_prepared = true; | 1171 | master->cur_msg_prepared = true; |
| 1169 | } | 1172 | } |
| @@ -1172,21 +1175,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread) | |||
| 1172 | if (ret) { | 1175 | if (ret) { |
| 1173 | master->cur_msg->status = ret; | 1176 | master->cur_msg->status = ret; |
| 1174 | spi_finalize_current_message(master); | 1177 | spi_finalize_current_message(master); |
| 1175 | mutex_unlock(&master->bus_lock_mutex); | 1178 | goto out; |
| 1176 | return; | ||
| 1177 | } | 1179 | } |
| 1178 | 1180 | ||
| 1179 | ret = master->transfer_one_message(master, master->cur_msg); | 1181 | ret = master->transfer_one_message(master, master->cur_msg); |
| 1180 | if (ret) { | 1182 | if (ret) { |
| 1181 | dev_err(&master->dev, | 1183 | dev_err(&master->dev, |
| 1182 | "failed to transfer one message from queue\n"); | 1184 | "failed to transfer one message from queue\n"); |
| 1183 | mutex_unlock(&master->bus_lock_mutex); | 1185 | goto out; |
| 1184 | return; | ||
| 1185 | } | 1186 | } |
| 1186 | mutex_unlock(&master->bus_lock_mutex); | 1187 | |
| 1188 | out: | ||
| 1189 | if (!bus_locked) | ||
| 1190 | mutex_unlock(&master->bus_lock_mutex); | ||
| 1187 | 1191 | ||
| 1188 | /* Prod the scheduler in case transfer_one() was busy waiting */ | 1192 | /* Prod the scheduler in case transfer_one() was busy waiting */ |
| 1189 | cond_resched(); | 1193 | if (!ret) |
| 1194 | cond_resched(); | ||
| 1190 | } | 1195 | } |
| 1191 | 1196 | ||
| 1192 | /** | 1197 | /** |
| @@ -1198,7 +1203,7 @@ static void spi_pump_messages(struct kthread_work *work) | |||
| 1198 | struct spi_master *master = | 1203 | struct spi_master *master = |
| 1199 | container_of(work, struct spi_master, pump_messages); | 1204 | container_of(work, struct spi_master, pump_messages); |
| 1200 | 1205 | ||
| 1201 | __spi_pump_messages(master, true); | 1206 | __spi_pump_messages(master, true, false); |
| 1202 | } | 1207 | } |
| 1203 | 1208 | ||
| 1204 | static int spi_init_queue(struct spi_master *master) | 1209 | 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, | |||
| 2462 | spi_sync_immediate); | 2467 | spi_sync_immediate); |
| 2463 | SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics, | 2468 | SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics, |
| 2464 | spi_sync_immediate); | 2469 | spi_sync_immediate); |
| 2465 | __spi_pump_messages(master, false); | 2470 | __spi_pump_messages(master, false, bus_locked); |
| 2466 | } | 2471 | } |
| 2467 | 2472 | ||
| 2468 | wait_for_completion(&done); | 2473 | wait_for_completion(&done); |
