Re: [PATCH v4] mtd: spi-nor: take the flash lock in spi_nor_restore()

[Itai Handler <itai.handler@gmail.com>]

On Sun, Oct 4, 2026 at 1:43 PM Itai Handler <itai.handler@gmail.com> wrote:
> Take the lock in spi_nor_restore(); both callers are teardown paths
> that can sleep and neither already holds it. Waiting is deliberate: a
> partially erased sector cannot be undone from here, and the wait is
> bounded by the one operation already in flight. If the lock cannot be
> taken the restore is skipped with a warning.

There are two mistakes here.

The wait is not bounded the way this suggests. spi_nor_erase() holds
the lock for the whole request, so shutdown or unbind can end up
waiting for a full chip erase.

And v4 still skips the restore when the lock cannot be taken, as v3
did. In my reply to Michael on v3 [1] I offered to do a best-effort
restore with a warning instead, but v4 only adds the warning:

> +	ret = spi_nor_prep_and_lock(nor);
> +	if (ret) {
> +		dev_warn(nor->dev,
> +			 "Failed to lock flash, skipping restore, err = %d\n",
> +			 ret);
> +		return;
> +	}

In practice that only fails on an RWW flash when the waiting task gets
a fatal signal, e.g. someone killing an unbind stuck behind a long
erase. The chip can then be left in 4-byte mode for whatever runs
next, which is exactly what this patch is meant to prevent.

Sashiko's review of v4 flagged both.

Please don't apply v4. I'll send v5 soon. It will still wait for the
request in flight, with a commit message that says how long that can
take. If the lock can't be taken it will warn and do the restore
anyway, as the code does today. That can race with the operation in
flight, but only after the waiting task has been killed.

[1] https://lore.kernel.org/r/CAFpOueRGN2TSNo+m3319qn00p-BYn8w7OQ6OPt4XTcVG63-uUA@mail.gmail.com

Thanks,
Itai

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/