| Message ID | 20170502000455.13240-1-computersforpeace@gmail.com |
|---|---|
| State | Accepted |
| Commit | 44d4182e23c555cbfa8b8a0ad2d94664d23850d3 |
| Delegated to: | Brian Norris |
| Headers | show |
On 1 May 2017 at 21:04, Brian Norris <computersforpeace@gmail.com> wrote: > This bug seems to have been here forever, although we came close to > fixing all of them in [1]! > > [1] 11eaf6df1cce ("mtd: nand: Remove BUG() abuse in nand_scan_tail") > Well, we came even closer. See [1] for a patch that fixes this by cleaning BBT init and release. Back then Boris suggested on IRC to wait for Peter Pan's BBT, which would address this too. It's interesting to note that: (1) the patch never got any formal feedback, despite it was a fix; and (2) Peter Pan's work is still under development. Guess I should have pressed the upstream buttons harder :-) [1] http://lists.infradead.org/pipermail/linux-mtd/2016-April/066834.html, [2] http://lists.infradead.org/pipermail/linux-mtd/2016-April/066835.html
On Mon, May 01, 2017 at 09:22:03PM -0300, Ezequiel Garcia wrote: > On 1 May 2017 at 21:04, Brian Norris <computersforpeace@gmail.com> wrote: > > This bug seems to have been here forever, although we came close to > > fixing all of them in [1]! > > > > [1] 11eaf6df1cce ("mtd: nand: Remove BUG() abuse in nand_scan_tail") > > > > Well, we came even closer. See [1] for a patch that fixes this by cleaning > BBT init and release. That's a different bug(fix), no? > Back then Boris suggested on IRC to wait for Peter Pan's BBT, which would > address this too. Haha, well bad strategy I guess :) > It's interesting to note that: (1) the patch never got any formal feedback, > despite it was a fix; and (2) Peter Pan's work is still under development. Yes, well you kinda nacked yourself, so (1) isn't that surprising. And (2) is well...no comment. > Guess I should have pressed the upstream buttons harder :-) I suppose. I'd be happy to see you resubmit, since I definitely would prioritize bugfixes over years-long refactoring. But I guess I'd defer to Boris, et al, who are paying much more attention to NAND stuff these days than I am. I just noticed this when bugfixing some things I noticed in Boris's pull request. Brian > [1] http://lists.infradead.org/pipermail/linux-mtd/2016-April/066834.html, > [2] http://lists.infradead.org/pipermail/linux-mtd/2016-April/066835.html > -- > Ezequiel García, VanguardiaSur > www.vanguardiasur.com.ar
On 1 May 2017 at 22:33, Brian Norris <computersforpeace@gmail.com> wrote: > On Mon, May 01, 2017 at 09:22:03PM -0300, Ezequiel Garcia wrote: >> On 1 May 2017 at 21:04, Brian Norris <computersforpeace@gmail.com> wrote: >> > This bug seems to have been here forever, although we came close to >> > fixing all of them in [1]! >> > >> > [1] 11eaf6df1cce ("mtd: nand: Remove BUG() abuse in nand_scan_tail") >> > >> >> Well, we came even closer. See [1] for a patch that fixes this by cleaning >> BBT init and release. > > That's a different bug(fix), no? > Oops, I really jumped on the gun here. The patch looks good. Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
On Mon, 1 May 2017 17:04:50 -0700 Brian Norris <computersforpeace@gmail.com> wrote: > This bug seems to have been here forever, although we came close to > fixing all of them in [1]! > > [1] 11eaf6df1cce ("mtd: nand: Remove BUG() abuse in nand_scan_tail") > > Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > Signed-off-by: Brian Norris <computersforpeace@gmail.com> Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > The goto isn't *really* necessary, but I thought it'd be more consistent. > > Compile tested only > > drivers/mtd/nand/nand_base.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 978242b1213f..e4919f9dece4 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -4794,7 +4794,11 @@ int nand_scan_tail(struct mtd_info *mtd) > return 0; > > /* Build bad block table */ > - return chip->scan_bbt(mtd); > + ret = chip->scan_bbt(mtd); > + if (ret) > + goto err_free; > + return 0; > + > err_free: > if (nbuf) { > kfree(nbuf->databuf);
On Mon, 1 May 2017 17:04:50 -0700 Brian Norris <computersforpeace@gmail.com> wrote: > This bug seems to have been here forever, although we came close to > fixing all of them in [1]! > > [1] 11eaf6df1cce ("mtd: nand: Remove BUG() abuse in nand_scan_tail") > > Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > Signed-off-by: Brian Norris <computersforpeace@gmail.com> Applied to nand/fixes. Thanks, Boris > --- > The goto isn't *really* necessary, but I thought it'd be more consistent. > > Compile tested only > > drivers/mtd/nand/nand_base.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 978242b1213f..e4919f9dece4 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -4794,7 +4794,11 @@ int nand_scan_tail(struct mtd_info *mtd) > return 0; > > /* Build bad block table */ > - return chip->scan_bbt(mtd); > + ret = chip->scan_bbt(mtd); > + if (ret) > + goto err_free; > + return 0; > + > err_free: > if (nbuf) { > kfree(nbuf->databuf);
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 978242b1213f..e4919f9dece4 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -4794,7 +4794,11 @@ int nand_scan_tail(struct mtd_info *mtd) return 0; /* Build bad block table */ - return chip->scan_bbt(mtd); + ret = chip->scan_bbt(mtd); + if (ret) + goto err_free; + return 0; + err_free: if (nbuf) { kfree(nbuf->databuf);
This bug seems to have been here forever, although we came close to fixing all of them in [1]! [1] 11eaf6df1cce ("mtd: nand: Remove BUG() abuse in nand_scan_tail") Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> Signed-off-by: Brian Norris <computersforpeace@gmail.com> --- The goto isn't *really* necessary, but I thought it'd be more consistent. Compile tested only drivers/mtd/nand/nand_base.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)