Hi Geert, On Tue, 22 Jan 2019 13:31:59 +0100 Boris Brezillon <bbrezillon@xxxxxxxxxx> wrote: > On Tue, 22 Jan 2019 12:21:11 +0100 > Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > > Hi Boris, > > > > On Wed, Jan 2, 2019 at 3:37 PM Boris Brezillon <bbrezillon@xxxxxxxxxx> wrote: > > > add_mtd_device() can fail. We should always check its return value > > > and gracefully handle the failure case. Fix the call sites where this > > > not done (in mtdpart.c) and add a __must_check attribute to the > > > prototype to avoid this kind of mistakes. > > > > > > Signed-off-by: Boris Brezillon <bbrezillon@xxxxxxxxxx> > > > --- > > > No Fixes or Cc-stable tag here, as this seems to have worked just fine > > > without checking add_mtd_device() ret code until we started to expose > > > MTD devices as NVMEM providers (queued for 4.21). > > > > Oh yes ;-) > > > > https://lore.kernel.org/lkml/87feac02-e955-1897-d4a4-d6d6d1082e45@xxxxxxxxx/t/ > > > > Your patch is very similar to mine, so the crash is gone. > > Oops, sorry about that. I completely forgot about this patch. It seems > the discussion led to a different conclusion though (patch > allocate_partitions() to reject wrong parts early) and the v2 was never > sent (or I missed it). Anyway, I guess we should have done both (check > add_mtd_device() ret code everywhere and patch allocate_partitions() to > reject bad parts early). > > > However, the warning is still there: > > > > m25p80 spi0.0: s25sl032p (4096 Kbytes) > > 3 fixed-partitions partitions found on MTD device spi0.0 > > Creating 3 MTD partitions on "spi0.0": > > 0x000000000000-0x000000080000 : "loader" > > 0x000000080000-0x000000600000 : "user" > > mtd: partition "user" extends beyond the end of device "spi0.0" -- > > size truncated to 0x380000 > > 0x000000600000-0x000004000000 : "flash" > > mtd: partition "flash" is out of reach -- disabled > > ------------[ cut here ]------------ > > WARNING: CPU: 0 PID: 1 at drivers/mtd/mtdcore.c:571 > > add_mtd_device+0x90/0x3b0 > > > > Interestingly, only one partition is created, covering the full size of the > > device: > > > > # cat /proc/partitions > > major minor #blocks name > > > > 31 0 4096 mtdblock0 > > > > While I would expect two partitions, "loader" and truncated "user": > > > > 31 0 512 mtdblock0 > > 31 1 3584 mtdblock1 > > Yes, makes sense, I guess your patch was better than mine :-/. Can you > try with the following diff applied and let me know if it solves the > problem? Gentle ping: is this diff fixing your problem, and do you want me to send a proper patch for it or should I let you send one? > > --->8--- > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > index 60104e1079c5..aefd3344991f 100644 > --- a/drivers/mtd/mtdpart.c > +++ b/drivers/mtd/mtdpart.c > @@ -724,16 +724,14 @@ int add_mtd_partitions(struct mtd_info *master, > { > struct mtd_part *slave; > uint64_t cur_offset = 0; > - int i, ret; > + int i, ret, actual_nbparts = 0; > > printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name); > > for (i = 0; i < nbparts; i++) { > slave = allocate_partition(master, parts + i, i, cur_offset); > - if (IS_ERR(slave)) { > - ret = PTR_ERR(slave); > - goto err_del_partitions; > - } > + if (IS_ERR(slave)) > + continue; > > mutex_lock(&mtd_partitions_mutex); > list_add(&slave->list, &mtd_partitions); > @@ -746,7 +744,7 @@ int add_mtd_partitions(struct mtd_info *master, > mutex_unlock(&mtd_partitions_mutex); > > free_partition(slave); > - goto err_del_partitions; > + continue; > } > > mtd_add_partition_attrs(slave); > @@ -754,14 +752,10 @@ int add_mtd_partitions(struct mtd_info *master, > parse_mtd_partitions(&slave->mtd, parts[i].types, NULL); > > cur_offset = slave->offset + slave->mtd.size; > + actual_nbparts++; > } > > - return 0; > - > -err_del_partitions: > - del_mtd_partitions(master); > - > - return ret; > + return actual_nbparts; > } > > static DEFINE_SPINLOCK(part_parser_lock); > @@ -1003,10 +997,10 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types, > } > /* Found partitions! */ > if (ret > 0) { > - err = add_mtd_partitions(master, pparts.parts, > + ret = add_mtd_partitions(master, pparts.parts, > pparts.nr_parts); > mtd_part_parser_cleanup(&pparts); > - return err ? err : pparts.nr_parts; > + return ret; > } > /* > * Stash the first error we see; only report it if no parser ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/