| Message ID | 20120904144826.7b9b7518@halley |
|---|---|
| State | New, archived |
| Headers | show |
于 2012年09月04日 19:48, Shmulik Ladkani 写道: > On Mon, 03 Sep 2012 18:35:44 +0300 Artem Bityutskiy<dedekind1@gmail.com> wrote: >> With the logic I suggested, for this case what I said would basically: >> >> 1. Sort. This leads to >> >> 100m@100m(kernel) >> 1g@200m(rootfs) >> 100m(boot) >> >> "OFFSET_CONTINOUS" is always the largest and will be at the end when >> sorting. >> >> 2. Verification. >> >> We verify for overlaps and gaps. And refuse this one. > Artem, Huang, > > Sorry for the long delay, got busy with urgent matters (found out I need > to go thru some surgery), I will probably not be available in the > upcoming days, but I've read the correspondance and wanted to share my > two cents... > > My POV is that sorting and verification is not needed, is troublesome, > and might affect users in ways they don't expect. > > So far, mtdparts commandline parsing has been very lenient and liberal. > I think we should keep this approach; give the user the flexibility, > he'll be responsible to provide meaningful cmdline parts for his > system. > > Actually, Huang's initial complaint was that 'parse_cmdline_partitions' > was too strict - the truncated partition was not registered! > > Now, philosophy aside, let's talk about some usecases that might break. > > I remember overlapping partitions (more precisely, partition that is a > subset of another) being common in some embedded systems, where the > "rootfs" and "kernel" partitions are a subset of the "overall image" > partition. > Why break this? if users enjoy the flexibility, and careful not to > create silly partitions, I'm in favor of keeping the flexibility. > > Same goes for sorting. > If one has a system hacked to work with mtd0 hardcodedly, but mtd0's > physical location is somewhere at the end of the device, why reorder the > partitions enumeration affecting this system? > > I'd say keep it simple and flexible. yes, it's really simple and flexible. But I think it's not wise to export the 0-size partition to user. The user may confused at this. And the mtd-utils, such as mtd-info/mtd-debug, do they can work with a 0-size partition? thanks Huang Shijie > I trust the user to provide meaningful partitions in the cmdline. > > Anyways, please consider this approach. > > My patch suggestion might look as (and might need some rework): > > diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c > index 17b0bd4..f5df613 100644 > --- a/drivers/mtd/cmdlinepart.c > +++ b/drivers/mtd/cmdlinepart.c > @@ -324,7 +324,6 @@ static int parse_cmdline_partitions(struct mtd_info *master, > "%s: partitioning exceeds flash size, truncating\n", > part->mtd_id); > part->parts[i].size = master->size - offset; > - part->num_parts = i; > } > offset += part->parts[i].size; > } > > Regards, > Shmulik > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ >
Huang Shijie <b32955 <at> freescale.com> writes: > 于 2012年09月04日 19:48, Shmulik Ladkani 写道: > > > > My POV is that sorting and verification is not needed, is troublesome, > > and might affect users in ways they don't expect. > > > > So far, mtdparts commandline parsing has been very lenient and liberal. > > I think we should keep this approach; give the user the flexibility, > > he'll be responsible to provide meaningful cmdline parts for his > > system. > > > > Actually, Huang's initial complaint was that 'parse_cmdline_partitions' > > was too strict - the truncated partition was not registered! > > > > Now, philosophy aside, let's talk about some usecases that might break. > > > > I remember overlapping partitions (more precisely, partition that is a > > subset of another) being common in some embedded systems, where the > > "rootfs" and "kernel" partitions are a subset of the "overall image" > > partition. > > Why break this? if users enjoy the flexibility, and careful not to > > create silly partitions, I'm in favor of keeping the flexibility. > > > > Same goes for sorting. > > If one has a system hacked to work with mtd0 hardcodedly, but mtd0's > > physical location is somewhere at the end of the device, why reorder the > > partitions enumeration affecting this system? > > > > I'd say keep it simple and flexible. > yes, it's really simple and flexible. Is it agreed that sorting is not necessary and will not be implemented? If so are there objections to me modifying the documentation to indicate that out of order partitions is part of the supported design? In my embedded distribution, I would like to introduce a MTD partition at the end of the list to not affect existing hardcoded MTD numbers. The fw_printenv/fw_setenv command line interface to U-Boot environment makes use of a /etc/fw_env.config with hardcoded device numbers which would be broken by a sort. Although my out of order partitions worked, I couldn't find any documentation indicating it was a supported feature. This thread with Artem's support for sorting got me worried. If the only reason for sorting is to prevent overlapping partitions, I think this can be written just as efficiently without a sort although Shmulik also gives a reason why this restriction should not exist. I'd prefer to accept that task rather than have to handle changing MTD numbers. > But I think it's not wise to export the 0-size partition to user. > The user may confused at this. Although I just submitted a patch to improve this, personally I think if a user specifies a zero sized partition, they should get it. Or if it's inherently incorrect I think it should be an error. Similarly I'd expect the same for truncated partitions, either provide the size specified or error. Since I don't ever foresee using either of these two features I don't have any objections to current situation. These are just my $0.02 -- Chris
(re-include others on CC) On Sun, Dec 16, 2012 at 5:11 PM, Christopher Cordahi <christophercordahi@nanometrics.ca> wrote: > Huang Shijie <b32955 <at> freescale.com> writes: >> 于 2012年09月04日 19:48, Shmulik Ladkani 写道: >> > >> > My POV is that sorting and verification is not needed, is troublesome, >> > and might affect users in ways they don't expect. >> > >> > So far, mtdparts commandline parsing has been very lenient and liberal. >> > I think we should keep this approach; give the user the flexibility, >> > he'll be responsible to provide meaningful cmdline parts for his >> > system. >> > >> > Actually, Huang's initial complaint was that 'parse_cmdline_partitions' >> > was too strict - the truncated partition was not registered! >> > >> > Now, philosophy aside, let's talk about some usecases that might break. >> > >> > I remember overlapping partitions (more precisely, partition that is a >> > subset of another) being common in some embedded systems, where the >> > "rootfs" and "kernel" partitions are a subset of the "overall image" >> > partition. >> > Why break this? if users enjoy the flexibility, and careful not to >> > create silly partitions, I'm in favor of keeping the flexibility. >> > >> > Same goes for sorting. >> > If one has a system hacked to work with mtd0 hardcodedly, but mtd0's >> > physical location is somewhere at the end of the device, why reorder the >> > partitions enumeration affecting this system? >> > >> > I'd say keep it simple and flexible. >> yes, it's really simple and flexible. > > Is it agreed that sorting is not necessary and will not be implemented? > If so are there objections to me modifying the documentation to indicate > that out of order partitions is part of the supported design? I don't object but rather would welcome the documentation. I think the consensus was that there were real reasons to allow unsorted partitions at least, and maybe even overlapping partitions. > In my embedded distribution, I would like to introduce a MTD partition > at the end of the list to not affect existing hardcoded MTD numbers. FWIW, my embedded systems make use of both out-of-order and overlapping partitions. I don't see why they must be sorted, and currently, overlapping is useful for me in exactly the scenarios given above (rootfs + kernel as subsets of entire_device). > Although my out of order partitions worked, I couldn't find any > documentation indicating it was a supported feature. This thread with > Artem's support for sorting got me worried. > > If the only reason for sorting is to prevent overlapping partitions, > I think this can be written just as efficiently without a sort although > Shmulik also gives a reason why this restriction should not exist. > I'd prefer to accept that task rather than have to handle changing > MTD numbers. > >> But I think it's not wise to export the 0-size partition to user. >> The user may confused at this. > > Although I just submitted a patch to improve this, personally I think > if a user specifies a zero sized partition, they should get it. Or if > it's inherently incorrect I think it should be an error. Similarly > I'd expect the same for truncated partitions, either provide the size > specified or error. Since I don't ever foresee using either of these > two features I don't have any objections to current situation. These > are just my $0.02 Well, the truncated partition is helpful sometimes. Perhaps an old flash partition layout is used when swapping in a different, smaller NAND; the truncated partition allows the system to still work, with a nice warning in case the user wants to fix it. Brian
On Mon, 2012-12-17 at 01:11 +0000, Christopher Cordahi wrote: > > > I'd say keep it simple and flexible. > > yes, it's really simple and flexible. > > Is it agreed that sorting is not necessary and will not be implemented? > If so are there objections to me modifying the documentation to indicate > that out of order partitions is part of the supported design? I think we agreed not to do this.
diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c index 17b0bd4..f5df613 100644 --- a/drivers/mtd/cmdlinepart.c +++ b/drivers/mtd/cmdlinepart.c @@ -324,7 +324,6 @@ static int parse_cmdline_partitions(struct mtd_info *master, "%s: partitioning exceeds flash size, truncating\n", part->mtd_id); part->parts[i].size = master->size - offset; - part->num_parts = i; } offset += part->parts[i].size; }