Skip to content

Commit 49023d2

Browse files
jonhunterbroonie
authored andcommitted
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 556351f ("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: 556351f ("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>
1 parent 6282697 commit 49023d2

File tree

1 file changed

+17
-12
lines changed

1 file changed

+17
-12
lines changed

drivers/spi/spi.c

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,6 +1047,7 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
10471047
* __spi_pump_messages - function which processes spi message queue
10481048
* @master: master to process queue for
10491049
* @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
10501051
*
10511052
* This function checks if there is any spi message in the queue that
10521053
* needs processing and if so call out to the driver to initialize hardware
@@ -1056,7 +1057,8 @@ EXPORT_SYMBOL_GPL(spi_finalize_current_transfer);
10561057
* inside spi_sync(); the queue extraction handling at the top of the
10571058
* function should deal with this safely.
10581059
*/
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)
10601062
{
10611063
unsigned long flags;
10621064
bool was_busy = false;
@@ -1152,7 +1154,9 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
11521154
}
11531155
}
11541156

1155-
mutex_lock(&master->bus_lock_mutex);
1157+
if (!bus_locked)
1158+
mutex_lock(&master->bus_lock_mutex);
1159+
11561160
trace_spi_message_start(master->cur_msg);
11571161

11581162
if (master->prepare_message) {
@@ -1162,8 +1166,7 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
11621166
"failed to prepare message: %d\n", ret);
11631167
master->cur_msg->status = ret;
11641168
spi_finalize_current_message(master);
1165-
mutex_unlock(&master->bus_lock_mutex);
1166-
return;
1169+
goto out;
11671170
}
11681171
master->cur_msg_prepared = true;
11691172
}
@@ -1172,21 +1175,23 @@ static void __spi_pump_messages(struct spi_master *master, bool in_kthread)
11721175
if (ret) {
11731176
master->cur_msg->status = ret;
11741177
spi_finalize_current_message(master);
1175-
mutex_unlock(&master->bus_lock_mutex);
1176-
return;
1178+
goto out;
11771179
}
11781180

11791181
ret = master->transfer_one_message(master, master->cur_msg);
11801182
if (ret) {
11811183
dev_err(&master->dev,
11821184
"failed to transfer one message from queue\n");
1183-
mutex_unlock(&master->bus_lock_mutex);
1184-
return;
1185+
goto out;
11851186
}
1186-
mutex_unlock(&master->bus_lock_mutex);
1187+
1188+
out:
1189+
if (!bus_locked)
1190+
mutex_unlock(&master->bus_lock_mutex);
11871191

11881192
/* Prod the scheduler in case transfer_one() was busy waiting */
1189-
cond_resched();
1193+
if (!ret)
1194+
cond_resched();
11901195
}
11911196

11921197
/**
@@ -1198,7 +1203,7 @@ static void spi_pump_messages(struct kthread_work *work)
11981203
struct spi_master *master =
11991204
container_of(work, struct spi_master, pump_messages);
12001205

1201-
__spi_pump_messages(master, true);
1206+
__spi_pump_messages(master, true, false);
12021207
}
12031208

12041209
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,
24622467
spi_sync_immediate);
24632468
SPI_STATISTICS_INCREMENT_FIELD(&spi->statistics,
24642469
spi_sync_immediate);
2465-
__spi_pump_messages(master, false);
2470+
__spi_pump_messages(master, false, bus_locked);
24662471
}
24672472

24682473
wait_for_completion(&done);

0 commit comments

Comments
 (0)