| Message ID | 20170417231003.7178-1-bart.vanassche@sandisk.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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@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? 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.
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
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.
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.)
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.
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;
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