[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@lists.infradead.org
Cc: linux-block@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

Re: [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@lists.infradead.org
> Cc: linux-block@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@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

Re: [PATCH] ARM: Fix rd_size declaration

[Bart Van Assche]

T24gVHVlLCAyMDE3LTA0LTE4IGF0IDA5OjM1ICswMjAwLCBKb2hhbm5lcyBUaHVtc2hpcm4gd3Jv
dGU6DQo+IE9uIE1vbiwgQXByIDE3LCAyMDE3IGF0IDA0OjEwOjAzUE0gLTA3MDAsIEJhcnQgVmFu
IEFzc2NoZSB3cm90ZToNCj4gPiBUaGUgZ2xvYmFsIHZhcmlhYmxlICdyZF9zaXplJyBpcyBkZWNs
YXJlZCBhcyAnaW50JyBpbiBzb3VyY2UgZmlsZQ0KPiA+IGFyY2gvYXJtL2tlcm5lbC9hdGFnc19w
YXJzZS5jIGFuZCBhcyAndW5zaWduZWQgbG9uZycgaW4NCj4gPiBkcml2ZXJzL2Jsb2NrL2JyZC5j
LiBGaXggdGhpcyBpbmNvbnNpc3RlbmN5Lg0KPiA+IA0KPiA+IFNpZ25lZC1vZmYtYnk6IEJhcnQg
VmFuIEFzc2NoZSA8YmFydC52YW5hc3NjaGVAc2FuZGlzay5jb20+DQo+ID4gQ2M6IFJ1c3NlbGwg
S2luZyA8bGludXhAYXJtbGludXgub3JnLnVrPg0KPiA+IENjOiBKZW5zIEF4Ym9lIDxheGJvZUBr
ZXJuZWwuZGs+DQo+ID4gQ2M6IEphbiBLYXJhIDxqYWNrQHN1c2UuY3o+DQo+ID4gQ2M6IDx5YW5h
aWppZUBodWF3ZWkuY29tPg0KPiA+IENjOiA8emhhb2hvbmdqaWFuZ0BodWF3ZWkuY29tPg0KPiA+
IENjOiA8bWlhb3hpZUBodWF3ZWkuY29tPg0KPiA+IENjOiBsaW51eC1hcm0ta2VybmVsQGxpc3Rz
LmluZnJhZGVhZC5vcmcNCj4gPiBDYzogbGludXgtYmxvY2tAdmdlci5rZXJuZWwub3JnDQo+ID4g
LS0tDQo+ID4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvYnJkLmggYi9pbmNsdWRlL2xpbnV4
L2JyZC5oDQo+ID4gbmV3IGZpbGUgbW9kZSAxMDA2NDQNCj4gPiBpbmRleCAwMDAwMDAwMDAwMDAu
LmRiYjBmOTJmZWZjOA0KPiA+IC0tLSAvZGV2L251bGwNCj4gPiArKysgYi9pbmNsdWRlL2xpbnV4
L2JyZC5oDQo+ID4gQEAgLTAsMCArMSBAQA0KPiA+ICtleHRlcm4gdW5zaWduZWQgbG9uZyByZF9z
aXplOw0KPiANCj4gU21hbGwgbml0LCBjYW4geW91IGFkZCBhbiBpbmNsdWRlIGd1YXJkIGhlcmUg
YXMgd2VsbD8NCg0KSGVsbG8gSm9oYW5uZXMsDQoNClRoYW5rcyBmb3IgdGhlIHJldmlldy4gQnV0
IGFyZSB5b3UgYXdhcmUgdGhhdCB3aXRoIHRoZSBjdXJyZW50IGNvbnRlbnQgYW4NCmluY2x1ZGUg
Z3VhcmQgaXMgb3ZlcmtpbGwgYmVjYXVzZSBpdCBpcyBzYWZlIHRvIGV2YWx1YXRlIHRoZSAiZXh0
ZXJuIHVuc2lnbmVkDQpsb25nIHJkX3NpemUiIGRlY2xhcmF0aW9uIG11bHRpcGxlIHRpbWVzPw0K
DQpCYXJ0Lg==

Re: [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@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

Re: [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.=

Re: [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.

Re: [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

[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@lists.infradead.org
Cc: linux-block@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

Re: [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.=

Re: [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.=

Re: [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.