On Mon, 26 Nov 2018 13:37:46 +0100
Boris Brezillon <boris.brezil...@bootlin.com> wrote:
> On Mon, 26 Nov 2018 16:42:48 +0530
> Jagan Teki <ja...@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.brezil...@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
