Hi Florin, Artem, On 5/3/07, Florin Malita <[EMAIL PROTECTED]> wrote:
[...] write_error: - kfree(new_seb); - /* May be this physical eraseblock went bad, try to pick another one */ - if (++tries <= 5) { + /* Maybe this physical eraseblock went bad, try to pick another one */ + if (++tries <= 5) err = ubi_scan_add_to_list(si, new_seb->pnum, new_seb->ec, &si->corr); - if (!err) - goto retry; - } + kfree(new_seb); + if (!err) + goto retry;
Eeks ... no, wait. You found a (two, actually) bug alright, but fixed it wrong. When we fail a write, we *must* add it to the corrupted list and _then_ attempt to retry. So, the "if (++tries <= 5)" applies to "if (!err) goto retry;" and not to the ubi_scan_add_to_list(). The difference is quite subtle here ... The correct fix should actually be as follows: (Artem, this is diffed on the original vtbl.c) --- Fix write_error logic in drivers/mtd/ubi/vtbl.c:create_vtbl(). On a write failure, we add the corrupted physical eraseblock to the corrupted list, and then attempt to retry (upto a maximum of 5 times). Also, fix an oops by pushing kfree(new_seb) below after dereferencing it for ubi_scan_add_to_list(). Signed-off-by: Satyam Sharma <[EMAIL PROTECTED]> Signed-off-by: Florin Malita <[EMAIL PROTECTED]> --- drivers/mtd/ubi/vtbl.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) --- diff -ruNp a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c --- a/drivers/mtd/ubi/vtbl.c 2007-04-20 18:42:17.000000000 +0530 +++ b/drivers/mtd/ubi/vtbl.c 2007-05-05 03:01:32.000000000 +0530 @@ -317,14 +317,12 @@ retry: return err; write_error: - kfree(new_seb); /* May be this physical eraseblock went bad, try to pick another one */ - if (++tries <= 5) { - err = ubi_scan_add_to_list(si, new_seb->pnum, new_seb->ec, - &si->corr); + err = ubi_scan_add_to_list(si, new_seb->pnum, new_seb->ec, &si->corr); + kfree(new_seb); + if (++tries <= 5) if (!err) goto retry; - } out_free: ubi_free_vid_hdr(ubi, vid_hdr); return err; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/