cmdlinepart.c patch

cmdlinepart.c patch

[Chuck Meade]

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

The cmdlinepart.c logic treats a partition offset that has been explicitly
specified as zero the same as an unspecified offset.  This causes the
partitioning logic to fail if your partition that starts at 0x0 is not the
first partition listed on the cmdline.

For example, if you have several partitions, and you want the third partition
to be an all-encompassing partition covering the whole device, then you
would give it offset 0x0.  The cmdlinepart.c logic treats this the same as
if you had not specified an offset, and instead tries to start the third
partition after the end of your second partition.  This is because the logic
uses zero as the "unspecified offset" value.  However, zero is not a good
choice for "unspecified offset" -- it is quite valid as a "specified" offset.

The attached patch defines the "unspecified offset" value as 0xffffffff and
makes the changes needed to support partitions that explicitly start at
offset 0x0, but are not necessarily listed first on the cmdline.

Chuck

[-- Attachment #2: cmdlinepart.c.patch --]
[-- Type: application/octet-stream, Size: 996 bytes --]

--- cmdlinepart.c.ORIG	2006-03-05 19:34:43.000000000 -0500
+++ cmdlinepart.c	2006-03-06 10:17:15.063475104 -0500
@@ -40,6 +40,10 @@
 #define dbg(x)
 #endif
 
+/* indicates that the offset for a partition was not specified, so start at
+ * end of previous partition.
+ */
+#define OFFSET_UNSPECIFIED 0xffffffff
 
 /* special size referring to all the remaining space in a partition */
 #define SIZE_REMAINING 0xffffffff
@@ -75,7 +79,7 @@ static struct mtd_partition * newpart(ch
 {
 	struct mtd_partition *parts;
 	unsigned long size;
-	unsigned long offset = 0;
+	unsigned long offset = OFFSET_UNSPECIFIED;
 	char *name;
 	int name_len;
 	unsigned char *extra_mem;
@@ -314,7 +318,7 @@ static int parse_cmdline_partitions(stru
 		{
 			for(i = 0, offset = 0; i < part->num_parts; i++)
 			{
-				if (!part->parts[i].offset)
+				if (part->parts[i].offset == OFFSET_UNSPECIFIED)
 				  part->parts[i].offset = offset;
 				else
 				  offset = part->parts[i].offset;

Re: cmdlinepart.c patch

[Jörn Engel]

On Mon, 6 March 2006 10:46:30 -0500, Chuck Meade wrote:
> 
> The cmdlinepart.c logic treats a partition offset that has been explicitly
> specified as zero the same as an unspecified offset.  This causes the
> partitioning logic to fail if your partition that starts at 0x0 is not the
> first partition listed on the cmdline.
> 
> For example, if you have several partitions, and you want the third partition
> to be an all-encompassing partition covering the whole device, then you
> would give it offset 0x0.  The cmdlinepart.c logic treats this the same as
> if you had not specified an offset, and instead tries to start the third
> partition after the end of your second partition.  This is because the logic
> uses zero as the "unspecified offset" value.  However, zero is not a good
> choice for "unspecified offset" -- it is quite valid as a "specified" offset.
> 
> The attached patch defines the "unspecified offset" value as 0xffffffff and
> makes the changes needed to support partitions that explicitly start at
> offset 0x0, but are not necessarily listed first on the cmdline.

I don't have any objections against your patch.  And seeing that it
was submitted at least three times without any other objections
either, it looks like it could go in.

And it also appears to indicate that few people care much about
cmdline.c. ;)

Any objections against inclusion?

Jörn

-- 
To recognize individual spam features you have to try to get into the
mind of the spammer, and frankly I want to spend as little time inside
the minds of spammers as possible.
-- Paul Graham

RE: cmdlinepart.c patch

[Chuck Meade]

> > ...
> > The attached patch defines the "unspecified offset" value as 0xffffffff and
> > makes the changes needed to support partitions that explicitly start at
> > offset 0x0, but are not necessarily listed first on the cmdline.
>
> I don't have any objections against your patch.  And seeing that it
> was submitted at least three times without any other objections
> either, it looks like it could go in.
>
> And it also appears to indicate that few people care much about
> cmdline.c. ;)
>
> Any objections against inclusion?

Hi Jörn,

I received email on this patch from Atsushi Nemoto, who submitted the earlier
patches for the same problem.  His patch has a couple of other nice features,
such as using UINT_MAX rather than 0xffffffff.  So if you want to use his
patch version that would be fine.

Thanks for getting back to me on this patch.

Regards,
Chuck

Re: cmdlinepart.c patch

[Jörn Engel]

On Sun, 12 March 2006 19:07:09 -0500, Chuck Meade wrote:
> 
> I received email on this patch from Atsushi Nemoto, who submitted the earlier
> patches for the same problem.  His patch has a couple of other nice features,
> such as using UINT_MAX rather than 0xffffffff.  So if you want to use his
> patch version that would be fine.

Can you send me his patch then?  Preferrably inline in the mail.

Jörn

-- 
I don't understand it. Nobody does.
-- Richard P. Feynman

RE: cmdlinepart.c patch

[Chuck Meade]

> Can you send me his patch then?  Preferrably inline in the mail.
>
> Jörn

Hi Jörn,

Yes.  I have included his patch inline below.  I got it from this
link that he sent:
http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.16-rc5/2.6.16-rc5-mm3/broken-out/mtd-cmdlinepart-allow-zero-offset-va
lue.patch

You may also be interested in this comment he made about this patch:

> I sent same fix to LKML (CC: David Woodhouse, Andrew Morton) on 29
> January.  And it has been in -mm tree since 2.6.16-rc1-mm4...

So just FYI this patch is already in -mm.

Below my signature is the inline version of the patch found at the
above link.

Thanks,
Chuck
----------------------------------------------------------------

From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

Current cmdlinepart.c uses offset value 0 to specify a continuous
partition.  This prevents creating a second partition starting at 0.

For example, I can split 4MB device using "mtdparts=id:2M,2M", but I can
not do "mtdparts=id:2M@2M,2M@0" to swap mtd0 and mtd1.

This patch introduces special OFFSET_CONTINUOUS value for a continuous
partition and allows 0 for offset value.

Also this patch replaces 0xffffffff with UINT_MAX for SIZE_REMAINING.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 drivers/mtd/cmdlinepart.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff -puN drivers/mtd/cmdlinepart.c~mtd-cmdlinepart-allow-zero-offset-value drivers/mtd/cmdlinepart.c
--- 25/drivers/mtd/cmdlinepart.c~mtd-cmdlinepart-allow-zero-offset-value	Wed Feb 22 15:25:21 2006
+++ 25-akpm/drivers/mtd/cmdlinepart.c	Wed Feb 22 15:25:21 2006
@@ -42,7 +42,8 @@


 /* special size referring to all the remaining space in a partition */
-#define SIZE_REMAINING 0xffffffff
+#define SIZE_REMAINING UINT_MAX
+#define OFFSET_CONTINUOUS UINT_MAX

 struct cmdline_mtd_partition {
 	struct cmdline_mtd_partition *next;
@@ -75,7 +76,7 @@ static struct mtd_partition * newpart(ch
 {
 	struct mtd_partition *parts;
 	unsigned long size;
-	unsigned long offset = 0;
+	unsigned long offset = OFFSET_CONTINUOUS;
 	char *name;
 	int name_len;
 	unsigned char *extra_mem;
@@ -314,7 +315,7 @@ static int parse_cmdline_partitions(stru
 		{
 			for(i = 0, offset = 0; i < part->num_parts; i++)
 			{
-				if (!part->parts[i].offset)
+				if (part->parts[i].offset == OFFSET_CONTINUOUS)
 				  part->parts[i].offset = offset;
 				else
 				  offset = part->parts[i].offset;
_

Re: cmdlinepart.c patch

[Jörn Engel]

On Mon, 13 March 2006 08:17:39 -0500, Chuck Meade wrote:
> 
> So just FYI this patch is already in -mm.

Great!  Then I'll happily ignore this patch and wait for the next
upstream merge. :)

Jörn

-- 
Simplicity is prerequisite for reliability.
-- Edsger W. Dijkstra