[PATCH] ARM: Fix rd_size declaration

[PATCH] ARM: Fix rd_size declaration

[Bart Van Assche]

The global variable 'rd_size' is declared as 'int' in source file
arch/arm/kernel/atags_parse.c and as 'unsigned long' in
drivers/block/brd.c. Fix this inconsistency.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Jan Kara <jack@suse.cz>
Cc: <yanaijie@huawei.com>
Cc: <zhaohongjiang@huawei.com>
Cc: <miaoxie@huawei.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-block at vger.kernel.org
---
 arch/arm/kernel/atags_parse.c | 4 ++--
 drivers/block/brd.c           | 1 +
 include/linux/brd.h           | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/brd.h

diff --git a/arch/arm/kernel/atags_parse.c b/arch/arm/kernel/atags_parse.c
index 68c6ae0b9e4c..85cb659e622a 100644
--- a/arch/arm/kernel/atags_parse.c
+++ b/arch/arm/kernel/atags_parse.c
@@ -23,6 +23,8 @@
 #include <linux/root_dev.h>
 #include <linux/screen_info.h>
 #include <linux/memblock.h>
+#include <linux/brd.h>		/* rd_size */
+#include <linux/initrd.h>	/* rd_image_start, rd_prompt, rd_doload */
 
 #include <asm/setup.h>
 #include <asm/system_info.h>
@@ -91,8 +93,6 @@ __tagtable(ATAG_VIDEOTEXT, parse_tag_videotext);
 #ifdef CONFIG_BLK_DEV_RAM
 static int __init parse_tag_ramdisk(const struct tag *tag)
 {
-	extern int rd_size, rd_image_start, rd_prompt, rd_doload;
-
 	rd_image_start = tag->u.ramdisk.start;
 	rd_doload = (tag->u.ramdisk.flags & 1) == 0;
 	rd_prompt = (tag->u.ramdisk.flags & 2) == 0;
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 3adc32a3153b..6d4bd38a9b7c 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -24,6 +24,7 @@
 #endif
 
 #include <linux/uaccess.h>
+#include <linux/brd.h>
 
 #define SECTOR_SHIFT		9
 #define PAGE_SECTORS_SHIFT	(PAGE_SHIFT - SECTOR_SHIFT)
diff --git a/include/linux/brd.h b/include/linux/brd.h
new file mode 100644
index 000000000000..dbb0f92fefc8
--- /dev/null
+++ b/include/linux/brd.h
@@ -0,0 +1 @@
+extern unsigned long rd_size;
-- 
2.12.2

[PATCH] ARM: Fix rd_size declaration

[Johannes Thumshirn]

On Mon, Apr 17, 2017 at 04:10:03PM -0700, Bart Van Assche wrote:
> The global variable 'rd_size' is declared as 'int' in source file
> arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> drivers/block/brd.c. Fix this inconsistency.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Jan Kara <jack@suse.cz>
> Cc: <yanaijie@huawei.com>
> Cc: <zhaohongjiang@huawei.com>
> Cc: <miaoxie@huawei.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-block at vger.kernel.org
> ---
> diff --git a/include/linux/brd.h b/include/linux/brd.h
> new file mode 100644
> index 000000000000..dbb0f92fefc8
> --- /dev/null
> +++ b/include/linux/brd.h
> @@ -0,0 +1 @@
> +extern unsigned long rd_size;

Small nit, can you add an include guard here as well?

Thanks,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH] ARM: Fix rd_size declaration

[Bart Van Assche]

On Tue, 2017-04-18 at 09:35 +0200, Johannes Thumshirn wrote:
> On Mon, Apr 17, 2017 at 04:10:03PM -0700, Bart Van Assche wrote:
> > The global variable 'rd_size' is declared as 'int' in source file
> > arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> > drivers/block/brd.c. Fix this inconsistency.
> > 
> > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Jens Axboe <axboe@kernel.dk>
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: <yanaijie@huawei.com>
> > Cc: <zhaohongjiang@huawei.com>
> > Cc: <miaoxie@huawei.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-block at vger.kernel.org
> > ---
> > diff --git a/include/linux/brd.h b/include/linux/brd.h
> > new file mode 100644
> > index 000000000000..dbb0f92fefc8
> > --- /dev/null
> > +++ b/include/linux/brd.h
> > @@ -0,0 +1 @@
> > +extern unsigned long rd_size;
> 
> Small nit, can you add an include guard here as well?

Hello Johannes,

Thanks for the review. But are you aware that with the current content an
include guard is overkill because it is safe to evaluate the "extern unsigned
long rd_size" declaration multiple times?

Bart.

[PATCH] ARM: Fix rd_size declaration

[Johannes Thumshirn]

On Tue, Apr 18, 2017 at 02:07:53PM +0000, Bart Van Assche wrote:
> Hello Johannes,
> 
> Thanks for the review. But are you aware that with the current content an
> include guard is overkill because it is safe to evaluate the "extern unsigned
> long rd_size" declaration multiple times?

Yes I am. But once someone does changes to this header and forgets the include
guard as well it may cause errors.

I am aware that this is rather cosmetic than needed, that's why I declared it
as a nit.

Byte,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH] ARM: Fix rd_size declaration

[Bart Van Assche]

On Mon, 2017-04-17 at 16:10 -0700, Bart Van Assche wrote:
> The global variable 'rd_size' is declared as 'int' in source file
> arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> drivers/block/brd.c. Fix this inconsistency.
> [ ... ]

Hello Russell,

Have I sent this patch to the right maintainer?

Thanks,

Bart.

[PATCH] ARM: Fix rd_size declaration

[Russell King - ARM Linux]

On Wed, Apr 26, 2017 at 08:51:35PM +0000, Bart Van Assche wrote:
> On Mon, 2017-04-17 at 16:10 -0700, Bart Van Assche wrote:
> > The global variable 'rd_size' is declared as 'int' in source file
> > arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> > drivers/block/brd.c. Fix this inconsistency.
> > [ ... ]
> 
> Hello Russell,
> 
> Have I sent this patch to the right maintainer?

There were comments on the patch which seemed to be unresolved.

I, too, don't like the idea of a single-line header file.  I'm also
wondering what the right solution here is - we're the only architecture
in the modern kernel that writes to rd_size, no one else does that.
I haven't been able to dig into the history to find out whether other
architectures used to, or what.

However, it would be nice if rd_size could go with some other related
declarations somewhere (if there are any.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: Fix rd_size declaration

[Bart Van Assche]

On Wed, 2017-05-03 at 20:25 +0100, Russell King - ARM Linux wrote:
> There were comments on the patch which seemed to be unresolved.

Do you mean the header guard? That's easy to resolve.

> However, it would be nice if rd_size could go with some other related
> declarations somewhere (if there are any.)

Sorry but I don't know about any other declarations that should be moved
into a brd header file.

Bart.

[PATCH] ARM: Fix rd_size declaration

[Russell King - ARM Linux]

On Wed, Jun 21, 2017 at 08:26:07PM +0000, Bart Van Assche wrote:
> On Wed, 2017-05-03 at 12:46 -0700, Bart Van Assche wrote:
> > The global variable 'rd_size' is declared as 'int' in source file
> > arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> > drivers/block/brd.c. Fix this inconsistency. Additionally, remove
> > the declarations of rd_image_start, rd_prompt and rd_doload from
> > parse_tag_ramdisk() since these duplicate existing declarations
> > in <linux/initrd.h>.
> 
> (replying to my own e-mail)
> 
> Hello Russell,
> 
> Did I send this patch to the right maintainer? If so, do you want to share
> any feedback about this patch?

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

I think it's fine from an ARM point of view.  It seems most of this patch
is to do with initrds, so I guess if no one else is interested in it,
akpm may pick it up.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: Fix rd_size declaration

[Bart Van Assche]

On Wed, 2017-05-03 at 12:46 -0700, Bart Van Assche wrote:
> The global variable 'rd_size' is declared as 'int' in source file
> arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> drivers/block/brd.c. Fix this inconsistency. Additionally, remove
> the declarations of rd_image_start, rd_prompt and rd_doload from
> parse_tag_ramdisk() since these duplicate existing declarations
> in <linux/initrd.h>.

(replying to my own e-mail)

Hello Russell,

Did I send this patch to the right maintainer? If so, do you want to share
any feedback about this patch?

Thanks,

Bart.

[PATCH] ARM: Fix rd_size declaration

[Bart Van Assche]

On Wed, 2017-05-03 at 12:46 -0700, Bart Van Assche wrote:
> The global variable 'rd_size' is declared as 'int' in source file
> arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> drivers/block/brd.c. Fix this inconsistency. Additionally, remove
> the declarations of rd_image_start, rd_prompt and rd_doload from
> parse_tag_ramdisk() since these duplicate existing declarations
> in <linux/initrd.h>.

This is version 2 of the patch for removing the rd_size declaration.
Compared to v1, the new header file <linux/brd.h> has been dropped.

Bart.

[PATCH] ARM: Fix rd_size declaration

[Bart Van Assche]

The global variable 'rd_size' is declared as 'int' in source file
arch/arm/kernel/atags_parse.c and as 'unsigned long' in
drivers/block/brd.c. Fix this inconsistency. Additionally, remove
the declarations of rd_image_start, rd_prompt and rd_doload from
parse_tag_ramdisk() since these duplicate existing declarations
in <linux/initrd.h>.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Jan Kara <jack@suse.cz>
Cc: <yanaijie@huawei.com>
Cc: <zhaohongjiang@huawei.com>
Cc: <miaoxie@huawei.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-block at vger.kernel.org
---
 arch/arm/kernel/atags_parse.c | 3 +--
 drivers/block/brd.c           | 1 +
 include/linux/initrd.h        | 3 +++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/atags_parse.c b/arch/arm/kernel/atags_parse.c
index 68c6ae0b9e4c..98fbfd235ac8 100644
--- a/arch/arm/kernel/atags_parse.c
+++ b/arch/arm/kernel/atags_parse.c
@@ -18,6 +18,7 @@
  */
 
 #include <linux/init.h>
+#include <linux/initrd.h>
 #include <linux/kernel.h>
 #include <linux/fs.h>
 #include <linux/root_dev.h>
@@ -91,8 +92,6 @@ __tagtable(ATAG_VIDEOTEXT, parse_tag_videotext);
 #ifdef CONFIG_BLK_DEV_RAM
 static int __init parse_tag_ramdisk(const struct tag *tag)
 {
-	extern int rd_size, rd_image_start, rd_prompt, rd_doload;
-
 	rd_image_start = tag->u.ramdisk.start;
 	rd_doload = (tag->u.ramdisk.flags & 1) == 0;
 	rd_prompt = (tag->u.ramdisk.flags & 2) == 0;
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 3adc32a3153b..30a45080a9b4 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/init.h>
+#include <linux/initrd.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/major.h>
diff --git a/include/linux/initrd.h b/include/linux/initrd.h
index 55289d261b4f..bc67b767f9ce 100644
--- a/include/linux/initrd.h
+++ b/include/linux/initrd.h
@@ -10,6 +10,9 @@ extern int rd_prompt;
 /* starting block # of image */
 extern int rd_image_start;
 
+/* size of a single RAM disk */
+extern unsigned long rd_size;
+
 /* 1 if it is not an error if initrd_start < memory_start */
 extern int initrd_below_start_ok;
 
-- 
2.12.2