Re: [RFC PATCH] mtd: rawnand: use mutex to protect access while in suspend

From: Sean Nyekjaer
Date: Tue Oct 05 2021 - 04:49:48 EST


On Tue, Oct 05, 2021 at 10:23:00AM +0200, Boris Brezillon wrote:
> On Tue, 5 Oct 2021 09:09:30 +0200
> Sean Nyekjaer <sean@xxxxxxxxxx> wrote:

[ ... ]

> >
> > Have you seen the reproducer script?
>
> How would I know about this script or your previous attempt (mentioned
> at the end of this email) given I was not Cc-ed on the previous
> discussion, and nothing mentions it in this RFC...
>

That's why I shared it here ;)
Initially I thought this was a bug introduced by exec_op.

> > ---
> > root@iwg26-v1:/data/root# cat /data/crash.sh
> > #!/bin/sh -x
> >
> > echo enabled > /sys/devices/platform/soc/2100000.bus/21f4000.serial/tty/ttymxc4/power/wakeup
> >
> > rm /data/test50M
> > dd if=/dev/urandom of=/tmp/test50M bs=1M count=50
> > cp /tmp/test50M /data/ &
> > sleep 1
> > echo mem > /sys/power/state
> > ---
> >
> > As seen in the log above disk is synced before suspend.
> > cp is continuing to copy data to ubifs.
> > And then user space processes are frozen.
> > At this point the kernel thread would have unwritten data.
> >
> > We tried to solve this with:
> > https://lkml.org/lkml/2021/9/1/280
>
> I see. It's still unclear to me when the write happens. Is it in the
> suspend path (before the system is actually suspended), or in the
> resume path (when the system is being resumed).
>
> Anyway, let's admit writing to a storage device while it's suspended is
> a valid use case and requires the storage layer to put this request on
> old. This wait should not, IMHO, be handled at the NAND level, but at
> the MTD level (using a waitqueue, and an atomic to make
> suspended/resumed transitions safe). And abusing a mutex to implement
> that is certainly not a good idea.

I did't say this was the right solution ;) I actually asked in the RFC:
"Should we introduce a new mutex? Or maybe a spin_lock?"

What are you proposing, a waitqueue in mtd_info? That gets checked in
mtd_write()/mtd_read()?

/Sean