* __setup()'s not processed in bk-current
@ 2004-06-28 21:34 Ricky Beam
2004-06-28 23:29 ` Ricky Beam
2004-06-28 23:57 ` Andrew Morton
0 siblings, 2 replies; 5+ messages in thread
From: Ricky Beam @ 2004-06-28 21:34 UTC (permalink / raw)
To: Linux Kernel Mail List
Bootdata ok (command line is ro console=ttyS0,115200 debug numa=off md=d0,0,2,0,
/dev/sda,/dev/sdb,/dev/sdc,/dev/sdd root=/dev/hdd1)
...
Built 1 zonelists
Kernel command line: ro console=ttyS0,115200 debug numa=off md=d0,0,2,0,/dev/sda
,/dev/sdb,/dev/sdc,/dev/sdd root=/dev/hdd1
Parsing ARGS: ro console=ttyS0,115200 debug numa=off md=d0,0,2,0,/dev/sda,/dev/s
db,/dev/sdc,/dev/sdd root=/dev/hdd1
parse_one(): params[0]: 8250.share_irqs
parse_one(): params[1]: 8250.probe_rsa
parse_one(): params[2]: scsi_mod.scsi_logging_level
parse_one(): params[3]: scsi_mod.max_luns
parse_one(): params[4]: scsi_mod.max_report_luns
parse_one(): params[5]: scsi_mod.inq_timeout
parse_one(): params[6]: scsi_mod.dev_flags
parse_one(): params[7]: scsi_mod.default_dev_flags
parse_one(): params[8]: usbcore.blinkenlights
parse_one(): params[9]: usbcore.usbfs_snoop
parse_one(): params[10]: ehci_hcd.log2_irq_thresh
parse_one(): params[11]: ohci_hcd.power_switching
parse_one(): params[12]: mousedev.xres
parse_one(): params[13]: mousedev.yres
parse_one(): params[14]: atkbd.set
parse_one(): params[15]: atkbd.reset
parse_one(): params[16]: atkbd.softrepeat
parse_one(): params[17]: atkbd.scroll
parse_one(): params[18]: atkbd.extra
parse_one(): params[19]: psmouse.proto
parse_one(): params[20]: psmouse.resolution
parse_one(): params[21]: psmouse.rate
parse_one(): params[22]: psmouse.smartscroll
parse_one(): params[23]: psmouse.resetafter
parse_one(): params[24]: i8042.noaux
parse_one(): params[25]: i8042.nomux
parse_one(): params[26]: i8042.unlock
parse_one(): params[27]: i8042.reset
parse_one(): params[28]: i8042.direct
parse_one(): params[29]: i8042.dumbkbd
Unknown argument: calling ffffffff805713c0
obsolete_checksetup(): line: console=ttyS0,115200
obsolete_checksetup(): checking: nosmp(5)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: H<83><EC>^XH<89>|H<8D>t$^TH<8D>|<E8>HQ<CD><FF>
<85><C0>t$HcD$^TH<C7><C7>+^N;<80><C7>^EN<D6><FA><FF>^A(47)
obsolete_checksetup(): checking: debug(5)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: H<89>=<91>k<F9><FF><BA>^A(9)
obsolete_checksetup(): checking: initcall_debug(14)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: <NULL>(0)
obsolete_checksetup(): checking: H<83><ECH<C7><C6>^E<B6>:<80>H<85><FF>H^OE<F7>1
<C0>H<C7><C7>!<B6>:<80><E8><B0><D8>^B(31)
obsolete_checksetup(): checking: load_ramdisk=(13)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: <80>?(2)
obsolete_checksetup(): checking: root=(5)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: <NULL>(1)
obsolete_checksetup(): checking: <B8>^A(2)
...
obsolete_checksetup(): checking: log_buf_len=(12)
...
Odd that it sees "log_buf_len=", but not the "console=" that's defined right
ahead of it:
(kernel/printk.o)
Contents of section .init.data:
0000 636f6e73 6f6c653d 006c6f67 5f627566 console=.log_buf
0010 5f6c656e 3d00 _len=.
init/main.o:
Contents of section .init.data:
0000 6e6f736d 70006d61 78637075 733d0070 nosmp.maxcpus=.p
0010 726f6669 6c653d00 64656275 67007175 rofile=.debug.qu
0020 69657400 696e6974 3d000000 00000000 iet.init=.......
0030 00000000 00000000 00000000 00000000 ................
...
0140 696e6974 63616c6c 5f646562 75670074 initcall_debug.t
0150 65737473 65747570 00746573 74736574 estsetup.testset
0160 75705f6c 6f6e6700 00000000 up_long.....
The linked kernel *looks* correct (x86_64 here):
Contents of section .init.data:
ffffffff8058d000 6e6f736d 70006d61 78637075 733d0070 nosmp.maxcpus=.p
Contents of section .init.setup:
ffffffff80593510 00d05880 ffffffff 60125780 ffffffff ..X.....`.W.....
ffffffff80593520 00000000 00000000 00000000 00000000 ................
ffffffff80593530 06d05880 ffffffff 70125780 ffffffff ..X.....p.W.....
ffffffff80593540 00000000 00000000 00000000 00000000 ................
Do I smell some bad pointer math? Yeap:
DEBUG: sizeof(obs_kernel_param): 24 (0x18)
obsolete_checksetup(): line: ro
obsolete_checksetup(): checking: nosmp(5) @ ffffffff80593510
obsolete_checksetup(): checking: <NULL>(1) @ ffffffff80593528
p++ moved the pointer sizeof(obs_kernel_param) ahead, but that's 8 bytes
short.
--Ricky
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: __setup()'s not processed in bk-current
2004-06-28 21:34 __setup()'s not processed in bk-current Ricky Beam
@ 2004-06-28 23:29 ` Ricky Beam
2004-06-28 23:57 ` Andrew Morton
1 sibling, 0 replies; 5+ messages in thread
From: Ricky Beam @ 2004-06-28 23:29 UTC (permalink / raw)
To: Linux Kernel Mail List
On Mon, 28 Jun 2004, Ricky Beam wrote:
>The linked kernel *looks* correct (x86_64 here):
>Contents of section .init.data:
> ffffffff8058d000 6e6f736d 70006d61 78637075 733d0070 nosmp.maxcpus=.p
>
>Contents of section .init.setup:
> ffffffff80593510 00d05880 ffffffff 60125780 ffffffff ..X.....`.W.....
> ffffffff80593520 00000000 00000000 00000000 00000000 ................
> ffffffff80593530 06d05880 ffffffff 70125780 ffffffff ..X.....p.W.....
> ffffffff80593540 00000000 00000000 00000000 00000000 ................
>
>Do I smell some bad pointer math? Yeap:
>DEBUG: sizeof(obs_kernel_param): 24 (0x18)
Ahh. Not bad pointer math. Bad kernel source :-) The compiler is not
aware of the packing/alignment of the .init.setup section until link
time which is too late. The .init.setup section is ALIGN(16)'d for
almost all archs. (h8300 is 4 bytes.)
There may be other instances like this that'll eventually bite us in the
ass.
(A) Fix... at least for x86_64.
===== include/linux/init.h 1.33 vs edited =====
--- 1.33/include/linux/init.h 2004-06-27 03:19:38 -04:00
+++ edited/include/linux/init.h 2004-06-28 18:39:37 -04:00
@@ -107,11 +107,12 @@
static initcall_t __initcall_##fn \
__attribute_used__ __attribute__((__section__(".security_initcall.init"))) = fn
+/* FIXME: not all arch's are aligned on a 16byte boundry */
struct obs_kernel_param {
const char *str;
int (*setup_func)(char *);
int early;
-};
+} __attribute__((aligned(16)));
/* Only for really core code. See moduleparam.h for the normal way. */
#define __setup_param(str, unique_id, fn, early) \
===== arch/x86_64/kernel/vmlinux.lds.S 1.23 vs edited =====
--- 1.23/arch/x86_64/kernel/vmlinux.lds.S 2004-05-30 07:33:25 -04:00
+++ edited/arch/x86_64/kernel/vmlinux.lds.S 2004-06-28 19:20:20 -04:00
@@ -87,7 +87,7 @@
_einittext = .;
}
.init.data : { *(.init.data) }
- . = ALIGN(16);
+ . = ALIGN(32);
__setup_start = .;
.init.setup : { *(.init.setup) }
__setup_end = .;
(don't ask me why __setup_start is landing 16b less than it should)
--Ricky
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: __setup()'s not processed in bk-current
2004-06-28 21:34 __setup()'s not processed in bk-current Ricky Beam
2004-06-28 23:29 ` Ricky Beam
@ 2004-06-28 23:57 ` Andrew Morton
2004-06-29 2:43 ` Rusty Russell
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2004-06-28 23:57 UTC (permalink / raw)
To: Ricky Beam; +Cc: linux-kernel, Andi Kleen, Rusty Russell
Ricky Beam <jfbeam@bluetronic.net> wrote:
>
> Do I smell some bad pointer math? Yeap:
> DEBUG: sizeof(obs_kernel_param): 24 (0x18)
>
> obsolete_checksetup(): line: ro
> obsolete_checksetup(): checking: nosmp(5) @ ffffffff80593510
> obsolete_checksetup(): checking: <NULL>(1) @ ffffffff80593528
>
> p++ moved the pointer sizeof(obs_kernel_param) ahead, but that's 8 bytes
> short.
Thanks for working that out. It's been handing around for ages.
We're now putting 24-byte structures into .init.setup via __setup. But
x86_64's compiler is emitting a `.align 16' in there, so they end up on
32-byte boundaries and do_early_param()'s pointer arithmetic goes wrong.
Fix that up by forcing the compiler to align these structures to sizeof(long).
---
25-akpm/include/linux/init.h | 1 +
1 files changed, 1 insertion(+)
diff -puN include/linux/init.h~x86_64-setup-section-alignment-fix include/linux/init.h
--- 25/include/linux/init.h~x86_64-setup-section-alignment-fix 2004-06-28 16:47:41.000000000 -0700
+++ 25-akpm/include/linux/init.h 2004-06-28 16:47:41.000000000 -0700
@@ -119,6 +119,7 @@ struct obs_kernel_param {
static struct obs_kernel_param __setup_##unique_id \
__attribute_used__ \
__attribute__((__section__(".init.setup"))) \
+ __attribute__((aligned((sizeof(long))))) \
= { __setup_str_##unique_id, fn, early }
#define __setup_null_param(str, unique_id) \
_
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: __setup()'s not processed in bk-current
2004-06-28 23:57 ` Andrew Morton
@ 2004-06-29 2:43 ` Rusty Russell
2004-06-29 4:40 ` Richard Henderson
0 siblings, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2004-06-29 2:43 UTC (permalink / raw)
To: Andrew Morton; +Cc: Ricky Beam, lkml - Kernel Mailing List, Andi Kleen, rth
On Tue, 2004-06-29 at 09:57, Andrew Morton wrote:
> We're now putting 24-byte structures into .init.setup via __setup. But
> x86_64's compiler is emitting a `.align 16' in there, so they end up on
> 32-byte boundaries and do_early_param()'s pointer arithmetic goes wrong.
>
> Fix that up by forcing the compiler to align these structures to sizeof(long).
Um, that's really odd, and at least deserves a comment.
There are a number of places where we assume that we can iterate through
all entries in a section as an array, rth would know if we've just been
lucky...
Thanks,
Rusty.
> diff -puN include/linux/init.h~x86_64-setup-section-alignment-fix include/linux/init.h
> --- 25/include/linux/init.h~x86_64-setup-section-alignment-fix 2004-06-28 16:47:41.000000000 -0700
> +++ 25-akpm/include/linux/init.h 2004-06-28 16:47:41.000000000 -0700
> @@ -119,6 +119,7 @@ struct obs_kernel_param {
> static struct obs_kernel_param __setup_##unique_id \
> __attribute_used__ \
> __attribute__((__section__(".init.setup"))) \
> + __attribute__((aligned((sizeof(long))))) \
> = { __setup_str_##unique_id, fn, early }
>
> #define __setup_null_param(str, unique_id) \
>
> _
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: __setup()'s not processed in bk-current
2004-06-29 2:43 ` Rusty Russell
@ 2004-06-29 4:40 ` Richard Henderson
0 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2004-06-29 4:40 UTC (permalink / raw)
To: Rusty Russell
Cc: Andrew Morton, Ricky Beam, lkml - Kernel Mailing List, Andi Kleen
On Tue, Jun 29, 2004 at 12:43:41PM +1000, Rusty Russell wrote:
> There are a number of places where we assume that we can iterate through
> all entries in a section as an array, rth would know if we've just been
> lucky...
You've been lucky.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2004-06-29 4:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-06-28 21:34 __setup()'s not processed in bk-current Ricky Beam
2004-06-28 23:29 ` Ricky Beam
2004-06-28 23:57 ` Andrew Morton
2004-06-29 2:43 ` Rusty Russell
2004-06-29 4:40 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox