From: Miquel Raynal <miquel.raynal@bootlin.com> To: Mark Brown <broonie@kernel.org>, <linux-spi@vger.kernel.org> Cc: Richard Weinberger <richard@nod.at>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Ryan Wanner <ryan.wanner@microchip.com>, Ronald Wahl <ronald.wahl@raritan.com>, David Laight <David.Laight@ACULAB.COM>, Nicolas Ferre <nicolas.ferre@microchip.com>, Claudiu Beznea <claudiu.beznea@tuxon.dev>, Alexandre Belloni <alexandre.belloni@bootlin.com>, <linux-arm-kernel@lists.infradead.org>, Miquel Raynal <miquel.raynal@bootlin.com>, stable@vger.kernel.org Subject: [PATCH] spi: atmel: Prevent spi transfers from being killed Date: Tue, 5 Dec 2023 09:31:02 +0100 [thread overview] Message-ID: <20231205083102.16946-1-miquel.raynal@bootlin.com> (raw) Upstream commit e0205d6203c2 ("spi: atmel: Prevent false timeouts on long transfers") has tried to mitigate the problem of getting spi transfers canceled because they were lasting too long. On slow buses, transfers in the MiB range can take more than one second and thus a calculation was added to progressively increment the timeout value. In order to not be too problematic from a user point of view (waiting dozen of seconds or even minutes), the wait call was turned interruptible. Turning the wait interruptible was a mistake as what we really wanted to do was to be able to kill a transfer. Any signal interrupting our transfer would not be suitable at all so a second attempt was made at turning the wait killable instead. Link: https://lore.kernel.org/linux-spi/20231127095842.389631-1-miquel.raynal@bootlin.com/ All being well, it was reported that JFFS2 was showing a splat when interrupting a transfer. After some more debate about whether JFFS2 should be fixed and how, it was also pointed out that the whole consistency of the filesystem in case of parallel I/O would be compromised. Changing JFFS2 behavior would in theory be possible but nobody has the energy and time and knowledge to do this now, so better prevent spi transfers to be interrupted by the user. Partially revert the blamed commit to no longer use the interruptible nor the killable variant of wait_for_completion(). Fixes: e0205d6203c2 ("spi: atmel: Prevent false timeouts on long transfers") Cc: stable@vger.kernel.org Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/spi/spi-atmel.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c index 0197c25f5029..54277de30161 100644 --- a/drivers/spi/spi-atmel.c +++ b/drivers/spi/spi-atmel.c @@ -1333,12 +1333,10 @@ static int atmel_spi_one_transfer(struct spi_controller *host, } dma_timeout = msecs_to_jiffies(spi_controller_xfer_timeout(host, xfer)); - ret_timeout = wait_for_completion_killable_timeout(&as->xfer_completion, - dma_timeout); - if (ret_timeout <= 0) { - dev_err(&spi->dev, "spi transfer %s\n", - !ret_timeout ? "timeout" : "canceled"); - as->done_status = ret_timeout < 0 ? ret_timeout : -EIO; + ret_timeout = wait_for_completion_timeout(&as->xfer_completion, dma_timeout); + if (!ret_timeout) { + dev_err(&spi->dev, "spi transfer timeout\n"); + as->done_status = -EIO; } if (as->done_status) -- 2.34.1
WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com> To: Mark Brown <broonie@kernel.org>, <linux-spi@vger.kernel.org> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>, Ryan Wanner <ryan.wanner@microchip.com>, Richard Weinberger <richard@nod.at>, stable@vger.kernel.org, Claudiu Beznea <claudiu.beznea@tuxon.dev>, David Laight <David.Laight@ACULAB.COM>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Miquel Raynal <miquel.raynal@bootlin.com>, Ronald Wahl <ronald.wahl@raritan.com>, linux-arm-kernel@lists.infradead.org Subject: [PATCH] spi: atmel: Prevent spi transfers from being killed Date: Tue, 5 Dec 2023 09:31:02 +0100 [thread overview] Message-ID: <20231205083102.16946-1-miquel.raynal@bootlin.com> (raw) Upstream commit e0205d6203c2 ("spi: atmel: Prevent false timeouts on long transfers") has tried to mitigate the problem of getting spi transfers canceled because they were lasting too long. On slow buses, transfers in the MiB range can take more than one second and thus a calculation was added to progressively increment the timeout value. In order to not be too problematic from a user point of view (waiting dozen of seconds or even minutes), the wait call was turned interruptible. Turning the wait interruptible was a mistake as what we really wanted to do was to be able to kill a transfer. Any signal interrupting our transfer would not be suitable at all so a second attempt was made at turning the wait killable instead. Link: https://lore.kernel.org/linux-spi/20231127095842.389631-1-miquel.raynal@bootlin.com/ All being well, it was reported that JFFS2 was showing a splat when interrupting a transfer. After some more debate about whether JFFS2 should be fixed and how, it was also pointed out that the whole consistency of the filesystem in case of parallel I/O would be compromised. Changing JFFS2 behavior would in theory be possible but nobody has the energy and time and knowledge to do this now, so better prevent spi transfers to be interrupted by the user. Partially revert the blamed commit to no longer use the interruptible nor the killable variant of wait_for_completion(). Fixes: e0205d6203c2 ("spi: atmel: Prevent false timeouts on long transfers") Cc: stable@vger.kernel.org Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/spi/spi-atmel.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c index 0197c25f5029..54277de30161 100644 --- a/drivers/spi/spi-atmel.c +++ b/drivers/spi/spi-atmel.c @@ -1333,12 +1333,10 @@ static int atmel_spi_one_transfer(struct spi_controller *host, } dma_timeout = msecs_to_jiffies(spi_controller_xfer_timeout(host, xfer)); - ret_timeout = wait_for_completion_killable_timeout(&as->xfer_completion, - dma_timeout); - if (ret_timeout <= 0) { - dev_err(&spi->dev, "spi transfer %s\n", - !ret_timeout ? "timeout" : "canceled"); - as->done_status = ret_timeout < 0 ? ret_timeout : -EIO; + ret_timeout = wait_for_completion_timeout(&as->xfer_completion, dma_timeout); + if (!ret_timeout) { + dev_err(&spi->dev, "spi transfer timeout\n"); + as->done_status = -EIO; } if (as->done_status) -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2023-12-05 8:31 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-12-05 8:31 Miquel Raynal [this message] 2023-12-05 8:31 ` [PATCH] spi: atmel: Prevent spi transfers from being killed Miquel Raynal 2023-12-05 9:22 ` Richard Weinberger 2023-12-05 9:22 ` Richard Weinberger 2023-12-05 9:28 ` Miquel Raynal 2023-12-05 9:28 ` Miquel Raynal 2023-12-05 9:23 ` Ronald Wahl 2023-12-05 9:23 ` Ronald Wahl 2023-12-05 15:41 ` Mark Brown 2023-12-05 15:41 ` Mark Brown
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231205083102.16946-1-miquel.raynal@bootlin.com \ --to=miquel.raynal@bootlin.com \ --cc=David.Laight@ACULAB.COM \ --cc=alexandre.belloni@bootlin.com \ --cc=broonie@kernel.org \ --cc=claudiu.beznea@tuxon.dev \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-spi@vger.kernel.org \ --cc=nicolas.ferre@microchip.com \ --cc=richard@nod.at \ --cc=ronald.wahl@raritan.com \ --cc=ryan.wanner@microchip.com \ --cc=stable@vger.kernel.org \ --cc=thomas.petazzoni@bootlin.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.