[PATCH] mtd: spi-nor: Reschedule in spi_nor_wait_till_ready()

[PATCH] mtd: spi-nor: Reschedule in spi_nor_wait_till_ready()

[Bart Van Assche]

In spi_nor_wait_till_ready_with_timeout(), cond_resched() is only
called after spi_nor_ready() returns 0. If the flash is already ready on
the first status check, the function returns before reaching
cond_resched(). This happens e.g. under QEMU, where SPI NOR page
programs and sector erases complete synchronously during the MMIO write,
combined with a controller driver that polls without sleeping.
Consequently, multi-sector erases in spi_nor_erase() and multi-page
writes in spi_nor_write() may iterate over thousands of sectors or pages
without ever yielding the CPU on kernels built without
CONFIG_PREEMPTION, e.g. ARM32 kernels with CONFIG_PREEMPT_VOLUNTARY=y.
This triggers soft lockup and RCU stall reports.

Move cond_resched() to the start of the polling loop such that every
call reschedules if needed, even if the flash is ready on the first
check. Doing this in the shared wait helper instead of in the
spi_nor_erase() and spi_nor_write() loops covers all callers that wait
for a program or erase operation to complete.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/mtd/spi-nor/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 90524c58602e..3e3b727d262e 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -653,6 +653,8 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
 	deadline = jiffies + timeout_jiffies;
 
 	while (!timeout) {
+		cond_resched();
+
 		if (time_after_eq(jiffies, deadline))
 			timeout = 1;
 
@@ -661,8 +663,6 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
 			return ret;
 		if (ret)
 			return 0;
-
-		cond_resched();
 	}
 
 	dev_dbg(nor->dev, "flash operation timed out\n");

Re: [PATCH] mtd: spi-nor: Reschedule in spi_nor_wait_till_ready()

[Bart Van Assche]

On 11/6/26 1:04 PM, Bart Van Assche wrote:
> In spi_nor_wait_till_ready_with_timeout(), cond_resched() is only
> called after spi_nor_ready() returns 0. If the flash is already ready on
> the first status check, the function returns before reaching
> cond_resched(). This happens e.g. under QEMU, where SPI NOR page
> programs and sector erases complete synchronously during the MMIO write,
> combined with a controller driver that polls without sleeping.
> Consequently, multi-sector erases in spi_nor_erase() and multi-page
> writes in spi_nor_write() may iterate over thousands of sectors or pages
> without ever yielding the CPU on kernels built without
> CONFIG_PREEMPTION, e.g. ARM32 kernels with CONFIG_PREEMPT_VOLUNTARY=y.
> This triggers soft lockup and RCU stall reports.
> 
> Move cond_resched() to the start of the polling loop such that every
> call reschedules if needed, even if the flash is ready on the first
> check. Doing this in the shared wait helper instead of in the
> spi_nor_erase() and spi_nor_write() loops covers all callers that wait
> for a program or erase operation to complete.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/mtd/spi-nor/core.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 90524c58602e..3e3b727d262e 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -653,6 +653,8 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
>   	deadline = jiffies + timeout_jiffies;
>   
>   	while (!timeout) {
> +		cond_resched();
> +
>   		if (time_after_eq(jiffies, deadline))
>   			timeout = 1;
>   
> @@ -661,8 +663,6 @@ static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
>   			return ret;
>   		if (ret)
>   			return 0;
> -
> -		cond_resched();
>   	}
>   
>   	dev_dbg(nor->dev, "flash operation timed out\n");

(+Brian)

The following probably should be added to the above patch:

Fixes: a95ce92e4b28 ("mtd: spi-nor: improve wait-till-ready timeout loop").

Bart.