[U-Boot] [PATCH v3 11/11] mtd: sf: Make sf_mtd.c more robust
Jagan Teki
jagan at openedev.com
Mon Nov 26 13:05:01 UTC 2018
On 26/11/18 6:12 PM, Boris Brezillon wrote:
> On Mon, 26 Nov 2018 13:37:46 +0100
> Boris Brezillon <boris.brezillon at bootlin.com> wrote:
>
>> On Mon, 26 Nov 2018 16:42:48 +0530
>> Jagan Teki <jagan at openedev.com> wrote:
>>
>>> On 26/11/18 2:12 PM, Boris Brezillon wrote:
>>>> Hi Jagan,
>>>>
>>>> On Thu, 22 Nov 2018 09:40:56 +0100
>>>> Boris Brezillon <boris.brezillon at bootlin.com> wrote:
>>>>
>>>>>>> + /*
>>>>>>> + * Setting mtd->priv to NULL is the best we can do. Thanks to that,
>>>>>>> + * the MTD layer can still call mtd hooks without risking a
>>>>>>> + * use-after-free bug. Still, things should be fixed to prevent the
>>>>>>> + * spi_flash object from being destroyed when del_mtd_device() fails.
>>>>>>> + */
>>>>>>> + sf_mtd_info.priv = NULL;
>>>>>>> + printf("Failed to unregister MTD %s and the spi_flash object is going away: you're in deep trouble!",
>>>>>>> + sf_mtd_info.name);
>>>>>>
>>>>>> Why do we need this print?
>>>>>
>>>>> Yes we do, just to keep the user informed that something bad happened
>>>>> and its spi-flash is no longer usable (at least through the MTD layer).
>>>>>
>>>>>> can't we do the same thing in MTD core
>>>>>> itself, so-that it can be generic for all flash objects.
>>>>>
>>>>> del_mtd_device() can fail, so it's the caller responsibility to decide
>>>>> what to do when that happens. Some users will propagate the error to
>>>>> the upper layer and maybe cancel the device removal (AFAICT,
>>>>> driver->remove() can return an error, not sure what happens in this
>>>>> case though). For others, like spi-flash, the device will go away, and
>>>>> all subsequent accesses will fail.
>>>>
>>>> I'm about to send a new version fixing the problem I mentioned in patch
>>>> 3, but before doing that, I'd like to know if my answer convinced you or
>>>> if you'd still prefer this message to go away (or be placed in
>>>> mtdcore/mtdpart.c).
>>>
>>>
>>> I'm thinking of having the message still in MTD by showing which
>>> interface it would belongs, along with the details.
>>
>> Then we'd need something less
>
> Sorry, I inadvertently hit the send button :-/.
>
> So, I was about to say that we need something less worrisome than the
> message I added in the SF layer if we move it to the MTD layer because
> some drivers might actually do the right thing when del_mtd_device()
> returns an error. I keep thinking that putting an error message in
> mtdcore.c is not the right thing to do, but if you insist, I'll add
> one (maybe a debug()). In any case I'd like to keep the one we have
> here, because in this specific case, there's simply nothing we can do
> when MTD dev removal fails.
>
Look like other flashes were deleting only via mtd_partitions. how would
they know and does it not need for them to print the same information?
More information about the U-Boot
mailing list