Re: [PATCH] mtd: spi-nor: Prefer asynchronous probe

From: Vignesh Raghavendra <vigneshr@ti.com>
To: Michael Walle <michael@walle.cc>, Doug Anderson <dianders@chromium.org>
Cc: richard@nod.at, tudor.ambarus@microchip.com,
	linux-mtd@lists.infradead.org,
	LKML <linux-kernel@vger.kernel.org>,
	miquel.raynal@bootlin.com
Subject: Re: [PATCH] mtd: spi-nor: Prefer asynchronous probe
Date: Mon, 5 Oct 2020 14:36:18 +0530	[thread overview]
Message-ID: <ccf3c162-4f2b-914f-725d-d15eb10819a4@ti.com> (raw)
In-Reply-To: <b376b3f2e41662237d50d589e3696b06@walle.cc>

Hi Michael,

On 10/3/20 10:24 PM, Michael Walle wrote:
> Hi Douglas,
> 
> Am 2020-10-03 18:27, schrieb Doug Anderson:
>> Hi,
>>
>> On Sat, Oct 3, 2020 at 8:22 AM Michael Walle <michael@walle.cc> wrote:
>>>
>>> Hi Douglas,
>>>
>>> > On my system the spi_nor_probe() took ~6 ms at bootup.  That's not a
>>> > lot, but every little bit adds up to a slow bootup.  While we can get
>>> > this out of the boot path by making it a module, there are times where
>>> > it is convenient (or even required) for this to be builtin the kernel.
>>> > Let's set that we prefer async probe so that we don't block other
>>> > drivers from probing while we are probing.
>>> >
>>> > This is a tiny little change that is almost guaranteed to be safe for
>>> > anything that is able to run as a module, which SPI_NOR is.
>>> > Specifically modules are already probed asynchronously.  Also: since
>>> > other things in the system may have enabled asynchronous probe the
>>> > system may already be doing other things during our probe.
>>> >
>>> > There is a small possibility that some other driver that was a client
>>> > of SPI_NOR didn't handle -EPROBE_DEFER and was relying on probe
>>> > ordering and only worked when the SPI_NOR and the SPI bus were
>>> > builtin.  In that case the other driver has a bug that's waiting to
>>> > hit and the other driver should be fixed.
>>>
>>> linux-next now triggers the following warning in kernel/kmod.c:136 on my
>>> board. I've bisected this to this patch.
>>>
[...]
>>
>> Thanks for your report!  My vote would be to revert my patch and then
>> this would need to be resolved before it could be added back in.
>> Without doing tons of research, maybe the right answer here is that
>> mtd_device_parse_register() should be moved into a separate task so
>> it's not blocking probe?  I probably won't try to tackle this
>> immediately, but the eventual goal is that async is default, so I
>> think this would need to be resolved before then.
> 
> Ok. Vignesh, will you take care of that?

Thanks for the report! I have posted a patch reverting this commit. Will
merge into spi-nor/next shortly

Regards
Vignesh

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