spi: core: Don't destroy master queue if we fail to create it
PATCH 1/1 spi: core: Don't destroy master queue if we fail to create it
From: Mark Brown <[email protected]>
If we fail to create the master queue for some reason we should not attempt to clean it up since attempting to stop a kthread that was not created will hang and it's just generally bad practice. Unfortunately at present we call spi_destroy_queue() even in cases where the creation fails.
Fix this by fixing the error handling in spi_master_initialize_queue() so that we only flag the master as queued or destroy the queue if creation succeeded. The change to the flag is done since the general master cleanup uses this to destroy the queue.
---
This has been compile tested only but it should hopefully be more robust in the long term than just skipping the queue deletion inside the destructor.
drivers/spi/spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/spi/spi.c +2 -2
@@ -1151,7 +1151,6 @@ static int spi_master_initialize_queue(struct spi_master *master) {int ret; - master->queued = true; master->transfer = spi_queued_transfer;if (!master->transfer_one_message) master->transfer_one_message = spi_transfer_one_message;
@@ -1162,6 +1161,7 @@ static int spi_master_initialize_queue(struct spi_master *master)dev_err (&master->dev,"problem initializing queue\n" ); goto err_init_queue; } + master->queued = true; ret =spi_start_queue (master);if (ret) {dev_err (&master->dev,"problem starting queue\n" );
@@ -1171,8 +1171,8 @@ static int spi_master_initialize_queue(struct spi_master *master)return 0 ; err_start_queue: -err_init_queue:spi_destroy_queue (master); +err_init_queue:return ret; }
Signature · 6 lines
1.9.2 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 1398966567-5182-1-git-send-email-broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Geert Uytterhoeven
Acked-byQuoted 36 lines · Mark Brown
From: Mark Brown <[email protected]>
If we fail to create the master queue for some reason we should not attempt to clean it up since attempting to stop a kthread that was not created will hang and it's just generally bad practice. Unfortunately at present we call
spi_destroy_queue()even in cases where the creation fails.Fix this by fixing the error handling in
spi_master_initialize_queue()so that we only flag the master as queued or destroy the queue if creation succeeded. The change to the flag is done since the general master cleanup uses this to destroy the queue.Reported-by Ricardo Ribalda Delgado <[email protected]>Signed-off-by Mark Brown <[email protected]>---
This has been compile tested only but it should hopefully be more robust in the long term than just skipping the queue deletion inside the destructor.
drivers/spi/spi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/spi/spi.c +0
-1@@ -1151,7 +1151,6 @@ static int spi_master_initialize_queue(struct spi_master *master) {int ret; - master->queued = true; master->transfer = spi_queued_transfer;if (!master->transfer_one_message) master->transfer_one_message = spi_transfer_one_message;
drivers/spi/spi.c static int spi_master_initialize_queue(struct spi_master *master) · line 1162
Quoted 2 lines
drivers/spi/spi.c +0
-0@@ -1162,6 +1161,7 @@ static int spi_master_initialize_queue(struct spi_master *master)dev_err (&master->dev,"problem initializing queue\n" ); goto err_init_queue;
You can just "return ret;" here...
drivers/spi/spi.c static int spi_master_initialize_queue(struct spi_master *master) · line 1175
Quoted 12 lines
drivers/spi/spi.c +2
-1} + master->queued = true; ret =spi_start_queue (master);if (ret) {dev_err (&master->dev,"problem starting queue\n" );@@ -1171,8 +1171,8 @@ static int spi_master_initialize_queue(struct spi_master *master)return 0 ; err_start_queue: -err_init_queue:spi_destroy_queue (master); +err_init_queue:
... and kill the err_init_queue label.
drivers/spi/spi.c static int spi_master_initialize_queue(struct spi_master *master) · line 1177
Quoted 2 lines
drivers/spi/spi.c +0
-0return ret; }
Apart from that:
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CAMuHMdXwX5He07h-peVPwgGYHUrPF5V7r9uH7s8BaaaFHMnQxw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 9 lines · Geert Uytterhoeven
drivers/spi/spi.c static int spi_master_initialize_queue(struct spi_master *master) · line 1162
Quoted 5 lines · Mark Brown
From: Mark Brown <[email protected]>
drivers/spi/spi.c +0
-0@@ -1162,6 +1161,7 @@ static int spi_master_initialize_queue(struct spi_master *master)dev_err (&master->dev,"problem initializing queue\n" ); goto err_init_queue;You can just "return ret;" here...
Quoted 6 lines
drivers/spi/spi.c static int spi_master_initialize_queue(struct spi_master *master) · line 1175
Quoted 4 lines
drivers/spi/spi.c +1
-1err_start_queue: -err_init_queue:spi_destroy_queue (master); +err_init_queue:... and kill the
err_init_queuelabel.
Yeah, I know. It's a stylistic thing - I tend to prefer to always use labels if they're used at all in a function even if we could return directly since it's more consistent. Not that I'm 100% consistent about that.
Quoted 2 lines
Apart from that:
Thanks.