From: Pratyush Yadav <pratyush@kernel.org>
To: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
Cc: "pratyush@kernel.org" <pratyush@kernel.org>,
"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>
Subject: Re: spi-nor: excessive locking in spi_nor_erase()
Date: Fri, 19 Apr 2024 14:58:03 +0200 [thread overview]
Message-ID: <mafs0y1995x2c.fsf@kernel.org> (raw)
In-Reply-To: <9bfdc3a36d4112dd6464f26ebe86e82e88cb6ccc.camel@infinera.com> (Joakim Tjernlund's message of "Wed, 17 Apr 2024 12:54:36 +0000")
On Wed, Apr 17 2024, Joakim Tjernlund wrote:
[...]
>> (spanning multiple sectors) is running, other tasks do still get to run
>> (since spi_nor_wait_till_ready() calls cond_resched()) but any task that
>> tries to access the rootfs would have to freeze since the lock is held.
>>
>> Dropping and re-acquiring the lock after erasing each sector or
>> programming each page would let readers make progress in between. This
>> shouldn't be too difficult to implement I reckon. You already mostly do
>> it in your patch below.
>
> Yes, that is simple.
>
>>
>> Doing erase or program suspend would only make sense for flashes with
>> larger sectors since even one sector erase would take a long time. For
>> those, I suppose we could give reads priority over program or erase
>> operations. When a read comes in, it suspends the erase, does the read,
>> and then resumes it. This would be a little bit more complex to
>> implement.
>
> Right, a bit harder but I think it is needed like cfi_cmdset_0001 do: suspend erase to do read/write
> Don't think cfi_cmdset suspends writes, that seems overkill.
>>
>> I would suggest you try the former first and see if it already gives you
>> the results you need. From your patch below, it seems it does. So
>> perhaps just cleaning it up and turning it onto a proper patch would do
>> the job for you.
>
> Tests with my patch shows vastly improved responsivity but there is still some
> way to go. Consider that an erase takes some 0.20-0.25 secs for us in best case in
> which apps are blocked. We do get some complaints from the app taking too long to complete
> some tasks during erase.
Send some patches to fix this then :-)
[...]
--
Regards,
Pratyush Yadav
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2024-04-19 12:58 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-15 15:55 spi-nor: excessive locking in spi_nor_erase() Joakim Tjernlund
2024-04-16 15:44 ` Pratyush Yadav
2024-04-16 18:03 ` Joakim Tjernlund
2024-04-17 12:42 ` Pratyush Yadav
2024-04-17 12:54 ` Joakim Tjernlund
2024-04-19 12:58 ` Pratyush Yadav [this message]
2024-04-19 13:14 ` Joakim Tjernlund
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=mafs0y1995x2c.fsf@kernel.org \
--to=pratyush@kernel.org \
--cc=Joakim.Tjernlund@infinera.com \
--cc=linux-mtd@lists.infradead.org \
/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: link
Be 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.