* ftrace breaks sparc64 build @ 2009-01-05 18:19 Sam Ravnborg 2009-01-05 19:31 ` Steven Rostedt 2009-01-05 19:48 ` ftrace breaks sparc64 build Al Viro 0 siblings, 2 replies; 37+ messages in thread From: Sam Ravnborg @ 2009-01-05 18:19 UTC (permalink / raw) To: LKML, Steven Rostedt, Ingo Molnar, rostedt, David S. Miller, sparclinux With an allmodconfig build on sparc and sparc64 I started to see warnings that become propagated to errors by -Werror. Example: CC arch/sparc/kernel/ldc.o arch/sparc/kernel/ldc.c: In function `process_control_frame': arch/sparc/kernel/ldc.c:627: warning: 'vap' might be used uninitialized in this function The code in question looks like this: static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) { struct ldc_version *vap; if ((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major))) { return ldc_abort(lp); } else { struct ldc_packet *p; unsigned long new_tail; p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, vap, sizeof(*vap), &new_tail); if (p) return send_tx_packet(lp, p, new_tail); else return ldc_abort(lp); } } The else part will never be executed whitout assigning vap, and this code do not emit warnings in the normal case. [I am well aware that we recommend to move the assignment out of the if () - but this code worked as is before]. This code gets expanded to: static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) { struct ldc_version *vap; if (__builtin_constant_p(((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major)))) ? !!((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major))) : ({ int ______r; static struct ftrace_branch_data ______f = { .func = __func__, .file = "arch/sparc/kernel/ldc.c", .line = 630, }; ______r = !!((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major))); if (______r) ______f.hit++; else ______f.miss++; ______r; })) { return ldc_abort(lp); } else { struct ldc_packet *p; unsigned long new_tail; p = handshake_compose_ctrl(lp, 0x01, 0x01, vap, sizeof(*vap), &new_tail); if (__builtin_constant_p((p)) ? !!(p) : ({ int ______r; static struct ftrace_branch_data ______f = { .func = __func__, .file = "arch/sparc/kernel/ldc.c", .line = 639, }; ______r = !!(p); if (______r) ______f.hit++; else ______f.miss++; ______r; })) return send_tx_packet(lp, p, new_tail); else return ldc_abort(lp); } } I have inserted newlines + tabs and removed a few __attribute__() to keep line lengths to a sensible level. My head started to spin with a dangerous speed trying to figure out the code snippet above. On top of this some inlining occurs which is why gcc point at another function name. This is with following gcc version: sparc64-unknown-linux-gnu-gcc (GCC) 3.4.5 Copyright (C) 2004 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Build using crosstool. Is this a known issue? Any recommendations? Sam ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 18:19 ftrace breaks sparc64 build Sam Ravnborg @ 2009-01-05 19:31 ` Steven Rostedt 2009-01-05 19:42 ` [PATCH] sparc: make proces_ver_nack a bit more readable Steven Rostedt 2009-01-05 19:54 ` ftrace breaks sparc64 build Sam Ravnborg 2009-01-05 19:48 ` ftrace breaks sparc64 build Al Viro 1 sibling, 2 replies; 37+ messages in thread From: Steven Rostedt @ 2009-01-05 19:31 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, 5 Jan 2009, Sam Ravnborg wrote: > With an allmodconfig build on sparc and sparc64 I started > to see warnings that become propagated to errors by -Werror. > > Example: > CC arch/sparc/kernel/ldc.o > arch/sparc/kernel/ldc.c: In function `process_control_frame': > arch/sparc/kernel/ldc.c:627: warning: 'vap' might be used uninitialized in this function > > > The code in question looks like this: > static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) > { > struct ldc_version *vap; > > if ((vp->major == 0 && vp->minor == 0) || > !(vap = find_by_major(vp->major))) { > return ldc_abort(lp); > } else { > struct ldc_packet *p; > unsigned long new_tail; > > p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, > vap, sizeof(*vap), > &new_tail); > if (p) > return send_tx_packet(lp, p, new_tail); > else > return ldc_abort(lp); > } > } > > The else part will never be executed whitout assigning vap, > and this code do not emit warnings in the normal case. > [I am well aware that we recommend to move the assignment > out of the if () - but this code worked as is before]. > > This code gets expanded to: > > static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) > { > struct ldc_version *vap; > > if (__builtin_constant_p(((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major)))) ? > !!((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major))) : > ({ > int ______r; > static struct ftrace_branch_data ______f = { .func = __func__, .file = "arch/sparc/kernel/ldc.c", .line = 630, }; > ______r = !!((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major))); > if (______r) > ______f.hit++; > else > ______f.miss++; ______r; > })) { > > return ldc_abort(lp); > } else { > struct ldc_packet *p; > unsigned long new_tail; > > p = handshake_compose_ctrl(lp, 0x01, 0x01, vap, sizeof(*vap), &new_tail); > if (__builtin_constant_p((p)) ? !!(p) : ({ > int ______r; > static struct ftrace_branch_data ______f = { .func = __func__, .file = "arch/sparc/kernel/ldc.c", .line = 639, }; > ______r = !!(p); > if (______r) > ______f.hit++; > else ______f.miss++; > ______r; > })) > return send_tx_packet(lp, p, new_tail); > else > return ldc_abort(lp); > } > } > I have inserted newlines + tabs and removed a few __attribute__() > to keep line lengths to a sensible level. > > My head started to spin with a dangerous speed trying to figure out > the code snippet above. My head spun a little by figuring out that vap is initialized in the original code. Honestly, that code is a little obfuscated, and would be better to write it as: if (vp->major == 0 && vp->minor=0) return ldc_abort(lp); vap = find_by_major(vp->major); if (!vap) return ldc_abort(lp); [...] This is much easier to read and we can remove the else statement altogether. And I bet the warning will go away if we did it this way. -- Steve > > On top of this some inlining occurs which is why gcc point at another > function name. > > > This is with following gcc version: > > sparc64-unknown-linux-gnu-gcc (GCC) 3.4.5 > Copyright (C) 2004 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > Build using crosstool. > > Is this a known issue? > > Any recommendations? > > Sam > > ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH] sparc: make proces_ver_nack a bit more readable 2009-01-05 19:31 ` Steven Rostedt @ 2009-01-05 19:42 ` Steven Rostedt 2009-01-05 19:46 ` Steven Rostedt ` (2 more replies) 2009-01-05 19:54 ` ftrace breaks sparc64 build Sam Ravnborg 1 sibling, 3 replies; 37+ messages in thread From: Steven Rostedt @ 2009-01-05 19:42 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux Impact: clean up The code in process_ver_nack is a little obfuscated. This change makes it a bit more readable by humans. It removes the complex if statement and replaces it with a cleaner flow of control. Signed-off-by: Steven Rostedt <srostedt@redhat.com> diff --git a/arch/sparc/kernel/ldc.c b/arch/sparc/kernel/ldc.c index d689823..6ce5d25 100644 --- a/arch/sparc/kernel/ldc.c +++ b/arch/sparc/kernel/ldc.c @@ -625,22 +625,23 @@ static int process_ver_ack(struct ldc_channel *lp, struct ldc_version *vp) static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) { struct ldc_version *vap; + struct ldc_packet *p; + unsigned long new_tail; - if ((vp->major == 0 && vp->minor == 0) || - !(vap = find_by_major(vp->major))) { + if (vp->major == 0 && vp->minor == 0) + return ldc_abort(lp); + + vap = find_by_major(vp->major); + if (!vap) return ldc_abort(lp); - } else { - struct ldc_packet *p; - unsigned long new_tail; - p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, + p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, vap, sizeof(*vap), &new_tail); - if (p) - return send_tx_packet(lp, p, new_tail); - else - return ldc_abort(lp); - } + if (!p) + return ldc_abort(lp); + + return send_tx_packet(lp, p, new_tail); } static int process_version(struct ldc_channel *lp, ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH] sparc: make proces_ver_nack a bit more readable 2009-01-05 19:42 ` [PATCH] sparc: make proces_ver_nack a bit more readable Steven Rostedt @ 2009-01-05 19:46 ` Steven Rostedt 2009-01-05 19:56 ` Steven Rostedt 2009-01-05 20:08 ` Sam Ravnborg 2 siblings, 0 replies; 37+ messages in thread From: Steven Rostedt @ 2009-01-05 19:46 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, 5 Jan 2009, Steven Rostedt wrote: > > Impact: clean up > > The code in process_ver_nack is a little obfuscated. This change > makes it a bit more readable by humans. It removes the complex > if statement and replaces it with a cleaner flow of control. Note, I do not have a sparc compiler at my disposal, so I was not able to compile (or boot) test this change. Here's the original code: static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) { struct ldc_version *vap; if ((vp->major == 0 && vp->minor == 0) || !(vap = find_by_major(vp->major))) { return ldc_abort(lp); } else { struct ldc_packet *p; unsigned long new_tail; p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, vap, sizeof(*vap), &new_tail); if (p) return send_tx_packet(lp, p, new_tail); else return ldc_abort(lp); } } And the new code: static int process_ver_nack(struct ldc_channel *lp, struct ldc_version *vp) { struct ldc_version *vap; struct ldc_packet *p; unsigned long new_tail; if (vp->major == 0 && vp->minor == 0) return ldc_abort(lp); vap = find_by_major(vp->major); if (!vap) return ldc_abort(lp); p = handshake_compose_ctrl(lp, LDC_INFO, LDC_VERS, vap, sizeof(*vap), &new_tail); if (!p) return ldc_abort(lp); return send_tx_packet(lp, p, new_tail); } This should be binary the same. But as I said, I do not have a sparc compiler to test. Thanks, -- Steve ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] sparc: make proces_ver_nack a bit more readable 2009-01-05 19:42 ` [PATCH] sparc: make proces_ver_nack a bit more readable Steven Rostedt 2009-01-05 19:46 ` Steven Rostedt @ 2009-01-05 19:56 ` Steven Rostedt 2009-01-05 20:07 ` Sam Ravnborg 2009-01-05 20:08 ` Sam Ravnborg 2 siblings, 1 reply; 37+ messages in thread From: Steven Rostedt @ 2009-01-05 19:56 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, 5 Jan 2009, Steven Rostedt wrote: > > Impact: clean up > > The code in process_ver_nack is a little obfuscated. This change > makes it a bit more readable by humans. It removes the complex > if statement and replaces it with a cleaner flow of control. > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> Sam, Can you test to see if this patch makes the issue go away. Obviously, Dave would need to be the one to pull it in, or at least ack it. Thanks, -- Steve ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] sparc: make proces_ver_nack a bit more readable 2009-01-05 19:56 ` Steven Rostedt @ 2009-01-05 20:07 ` Sam Ravnborg 0 siblings, 0 replies; 37+ messages in thread From: Sam Ravnborg @ 2009-01-05 20:07 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, Jan 05, 2009 at 02:56:58PM -0500, Steven Rostedt wrote: > > On Mon, 5 Jan 2009, Steven Rostedt wrote: > > > > > Impact: clean up > > > > The code in process_ver_nack is a little obfuscated. This change > > makes it a bit more readable by humans. It removes the complex > > if statement and replaces it with a cleaner flow of control. > > > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> > > Sam, > > Can you test to see if this patch makes the issue go away. Obviously, Dave > would need to be the one to pull it in, or at least ack it. As expected the patch silence the warning. The warning only triggers when we have an assignment after a boolean || inside an if condition. Sam ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] sparc: make proces_ver_nack a bit more readable 2009-01-05 19:42 ` [PATCH] sparc: make proces_ver_nack a bit more readable Steven Rostedt 2009-01-05 19:46 ` Steven Rostedt 2009-01-05 19:56 ` Steven Rostedt @ 2009-01-05 20:08 ` Sam Ravnborg 2009-01-06 18:23 ` David Miller 2 siblings, 1 reply; 37+ messages in thread From: Sam Ravnborg @ 2009-01-05 20:08 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, Jan 05, 2009 at 02:42:59PM -0500, Steven Rostedt wrote: > > Impact: clean up > > The code in process_ver_nack is a little obfuscated. This change > makes it a bit more readable by humans. It removes the complex > if statement and replaces it with a cleaner flow of control. > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> Reviewed-by: Sam Ravnborg <sam@ravnborg.org> And I can confirm the warning has dismissed with this patch. Sam ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] sparc: make proces_ver_nack a bit more readable 2009-01-05 20:08 ` Sam Ravnborg @ 2009-01-06 18:23 ` David Miller 0 siblings, 0 replies; 37+ messages in thread From: David Miller @ 2009-01-06 18:23 UTC (permalink / raw) To: sam; +Cc: rostedt, linux-kernel, srostedt, mingo, sparclinux From: Sam Ravnborg <sam@ravnborg.org> Date: Mon, 5 Jan 2009 21:08:48 +0100 > On Mon, Jan 05, 2009 at 02:42:59PM -0500, Steven Rostedt wrote: > > > > Impact: clean up > > > > The code in process_ver_nack is a little obfuscated. This change > > makes it a bit more readable by humans. It removes the complex > > if statement and replaces it with a cleaner flow of control. > > > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > And I can confirm the warning has dismissed with this patch. Applied, thanks everyone. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 19:31 ` Steven Rostedt 2009-01-05 19:42 ` [PATCH] sparc: make proces_ver_nack a bit more readable Steven Rostedt @ 2009-01-05 19:54 ` Sam Ravnborg 2009-01-05 20:05 ` Steven Rostedt 2009-01-05 20:30 ` [PATCH] module: clean up initialization of variable Steven Rostedt 1 sibling, 2 replies; 37+ messages in thread From: Sam Ravnborg @ 2009-01-05 19:54 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux Hi Steven. > > Honestly, that code is a little obfuscated, and would be better to write > it as: > > if (vp->major == 0 && vp->minor=0) > return ldc_abort(lp); > > vap = find_by_major(vp->major); > if (!vap) > return ldc_abort(lp); > > [...] > > This is much easier to read and we can remove the else statement > altogether. And I bet the warning will go away if we did it this way. Fully ageed on the readability. I happen to trigger this as an error in the sparc code. But I see the same warning also in generic code. >From kernel/module.c: /* Suck in entire file: we'll want most of it. */ /* vmalloc barfs on "unusual" numbers. Check here */ if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL) return ERR_PTR(-ENOMEM); This gives following warning: kernel/module.c: In function `load_module': kernel/module.c:1842: warning: 'hdr' might be used uninitialized in this function So this is not a pattern we seen only in sparc code and I wonder if this is the first time it is brought up? I can fix up the cases in sparc - no problem. But it was a suprise to me _why_ these warnings started to creep up and then it break my build. Sam ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 19:54 ` ftrace breaks sparc64 build Sam Ravnborg @ 2009-01-05 20:05 ` Steven Rostedt 2009-01-05 21:31 ` Sam Ravnborg 2009-01-06 18:32 ` David Miller 2009-01-05 20:30 ` [PATCH] module: clean up initialization of variable Steven Rostedt 1 sibling, 2 replies; 37+ messages in thread From: Steven Rostedt @ 2009-01-05 20:05 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, 5 Jan 2009, Sam Ravnborg wrote: > Hi Steven. > > > > > Honestly, that code is a little obfuscated, and would be better to write > > it as: > > > > if (vp->major == 0 && vp->minor=0) > > return ldc_abort(lp); > > > > vap = find_by_major(vp->major); > > if (!vap) > > return ldc_abort(lp); > > > > [...] > > > > This is much easier to read and we can remove the else statement > > altogether. And I bet the warning will go away if we did it this way. > > Fully ageed on the readability. > I happen to trigger this as an error in the sparc code. > But I see the same warning also in generic code. > > >From kernel/module.c: > /* Suck in entire file: we'll want most of it. */ > /* vmalloc barfs on "unusual" numbers. Check here */ > if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL) > return ERR_PTR(-ENOMEM); > > > This gives following warning: > kernel/module.c: In function `load_module': > kernel/module.c:1842: warning: 'hdr' might be used uninitialized in this function Probably the same issue. The problem is that the first use of a variable is in the OR section of an if statement that does a return. if (x || !(y = init_me()) return; use_me(y); IMHO I find this sloppy code. When reading the code it can cause reviewers trouble, and wasted time, to see that y is indeed initialized. I'm impressed that gcc was able to figure it out. > > So this is not a pattern we seen only in sparc code and I wonder if this is > the first time it is brought up? > > I can fix up the cases in sparc - no problem. > But it was a suprise to me _why_ these warnings started to creep > up and then it break my build. Have you always been compiling with -Werror? The reason that gcc complains is because you have the "branch_tracer" on that converts 'if ()' into a macro (as you saw in your -E compile). This makes the if statement more complex, and goes beyond gcc's ability to know that the above 'y' is initialized properly. I would work on fixing this in the branch tracer, but honestly, I'm kind of glad that gcc barfs on it. This will help us point out this kind of sloppy initializations (sorry if I'm offending anybody about calling it sloppy). I just believe that it makes the code a bit more obfuscated to initialize in an if statement, and a second part of a complex if statement at that! -- Steve ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 20:05 ` Steven Rostedt @ 2009-01-05 21:31 ` Sam Ravnborg 2009-01-05 21:52 ` Steven Rostedt 2009-01-06 18:32 ` David Miller 1 sibling, 1 reply; 37+ messages in thread From: Sam Ravnborg @ 2009-01-05 21:31 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux > > Probably the same issue. The problem is that the first use of a variable > is in the OR section of an if statement that does a return. > > if (x || !(y = init_me()) > return; > > use_me(y); > More warnings: fs/partitions/check.c: In function `rescan_partitions': fs/partitions/check.c:511: warning: 'state' might be used uninitialized in this function => follow same pattern fs/compat_ioctl.c: In function `ppp_sock_fprog_ioctl_trans': fs/compat_ioctl.c:859: warning: 'fptr32' might be used uninitialized in this function fs/compat_ioctl.c: In function `do_fontx_ioctl': fs/compat_ioctl.c:1082: warning: 'data' might be used uninitialized in this function fs/compat_ioctl.c: In function `do_atmif_sioc': fs/compat_ioctl.c:1302: warning: 'data' might be used uninitialized in this function fs/compat_ioctl.c: In function `do_atm_ioctl': fs/compat_ioctl.c:1272: warning: 'data' might be used uninitialized in this function fs/compat_ioctl.c: In function `mtd_rw_oob': fs/compat_ioctl.c:1421: warning: 'data' might be used uninitialized in this function fs/compat_ioctl.c: In function `do_usbdevfs_discsignal': fs/compat_ioctl.c:1650: warning: 'uctx' might be used uninitialized in this function This is: if (get_user(flen, &u_fprog32->len) || get_user(fptr32, &u_fprog32->filter)) return -EFAULT; which is a common pattern. I have at least one in sparc64 code too. I did not come up with a nice solution on this one. drivers/char/vt_ioctl.c: In function `vt_ioctl': drivers/char/vt_ioctl.c:945: warning: 'cc' might be used uninitialized in this function get_user() || get_user() again. net/core/skbuff.c: In function `___pskb_trim': net/core/skbuff.c:1041: warning: 'err' might be used uninitialized in this function if (xxx && err = foo()) net/core/dev.c: In function `skb_gso_segment': net/core/dev.c:1537: warning: 'err' might be used uninitialized in this function net/core/dev.c: In function `netif_receive_skb': net/core/dev.c:2075: warning: 'port' might be used uninitialized in this function if (xx && err = ...) if (xx || port = ...) net/core/neighbour.c: In function `neigh_resolve_output': net/core/neighbour.c:1188: warning: 'neigh' might be used uninitialized in this function net/core/neighbour.c: In function `neigh_create': net/core/neighbour.c:411: warning: 'error' might be used uninitialized in this function if (xx || neigh = ...) net/ipv4/ip_output.c: In function `ip_append_data': net/ipv4/ip_output.c:1006: warning: 'left' might be used uninitialized in this function net/ipv4/tcp.c: In function `tcp_sendpage': net/ipv4/tcp.c:687: warning: 'copy' might be used uninitialized in this function net/ipv4/tcp.c: In function `tcp_sendmsg': net/ipv4/tcp.c:862: warning: 'copy' might be used uninitialized in this function net/ipv4/tcp.c: In function `tcp_recvmsg': net/ipv4/tcp.c:1598: warning: 'chunk' might be used uninitialized in this function net/ipv4/tcp_ipv4.c: In function `listening_get_next': net/ipv4/tcp_ipv4.c:1874: warning: 'node' might be used uninitialized in this function net/ipv4/tcp_ipv4.c: In function `established_get_next': net/ipv4/tcp_ipv4.c:2012: warning: 'node' might be used uninitialized in this function net/ipv4/raw.c: In function `__raw_v4_lookup': net/ipv4/raw.c:113: warning: 'node' might be used uninitialized in this function net/ipv4/udp.c: In function `__udp4_lib_rcv': net/ipv4/udp.c:325: warning: 'node' might be used uninitialized in this function net/ipv4/udp.c:325: warning: 'node' might be used uninitialized in this function net/ipv4/devinet.c: In function `inet_gifconf': net/ipv4/devinet.c:821: warning: 'ifa' might be used uninitialized in this function net/ipv4/ipmr.c: In function `ipmr_get_route': net/ipv4/ipmr.c:1629: warning: 'vif' might be used uninitialized in this function net/ipv4/syncookies.c: In function `cookie_v4_check': net/ipv4/syncookies.c:263: warning: 'mss' might be used uninitialized in this function I have two warnings in sparc code I will deal with. One of them is the get_user() || get_user() pattern so I dunno how to fix it atm. The above is from a: make ARCH=sparc64 allmodconfig make ARCH=sparc64 vmlinux I did not try to build modules... Sam ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 21:31 ` Sam Ravnborg @ 2009-01-05 21:52 ` Steven Rostedt 2009-01-05 22:01 ` Sam Ravnborg 0 siblings, 1 reply; 37+ messages in thread From: Steven Rostedt @ 2009-01-05 21:52 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, 5 Jan 2009, Sam Ravnborg wrote: > > > > Probably the same issue. The problem is that the first use of a variable > > is in the OR section of an if statement that does a return. > > > > if (x || !(y = init_me()) > > return; > > > > use_me(y); > > > > More warnings: > fs/partitions/check.c: In function `rescan_partitions': > fs/partitions/check.c:511: warning: 'state' might be used uninitialized in this function > > => follow same pattern > > fs/compat_ioctl.c: In function `ppp_sock_fprog_ioctl_trans': > fs/compat_ioctl.c:859: warning: 'fptr32' might be used uninitialized in this function > fs/compat_ioctl.c: In function `do_fontx_ioctl': > fs/compat_ioctl.c:1082: warning: 'data' might be used uninitialized in this function > fs/compat_ioctl.c: In function `do_atmif_sioc': > fs/compat_ioctl.c:1302: warning: 'data' might be used uninitialized in this function > fs/compat_ioctl.c: In function `do_atm_ioctl': > fs/compat_ioctl.c:1272: warning: 'data' might be used uninitialized in this function > fs/compat_ioctl.c: In function `mtd_rw_oob': > fs/compat_ioctl.c:1421: warning: 'data' might be used uninitialized in this function > fs/compat_ioctl.c: In function `do_usbdevfs_discsignal': > fs/compat_ioctl.c:1650: warning: 'uctx' might be used uninitialized in this function > > This is: > if (get_user(flen, &u_fprog32->len) || > get_user(fptr32, &u_fprog32->filter)) > return -EFAULT; Is this all sparc cross compiler? I'm trying to reproduce it on x86 with no avail :-( I would like to test other ways to change the macro, but to do so, I need to get a compiler that will produce the warnings that you see. What version of gcc are you using? Thanks, -- Steve ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 21:52 ` Steven Rostedt @ 2009-01-05 22:01 ` Sam Ravnborg 2009-01-05 22:14 ` Steven Rostedt 0 siblings, 1 reply; 37+ messages in thread From: Sam Ravnborg @ 2009-01-05 22:01 UTC (permalink / raw) To: Steven Rostedt Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, Jan 05, 2009 at 04:52:02PM -0500, Steven Rostedt wrote: > > On Mon, 5 Jan 2009, Sam Ravnborg wrote: > > > > > > > Probably the same issue. The problem is that the first use of a variable > > > is in the OR section of an if statement that does a return. > > > > > > if (x || !(y = init_me()) > > > return; > > > > > > use_me(y); > > > > > > > More warnings: > > fs/partitions/check.c: In function `rescan_partitions': > > fs/partitions/check.c:511: warning: 'state' might be used uninitialized in this function > > > > => follow same pattern > > > > fs/compat_ioctl.c: In function `ppp_sock_fprog_ioctl_trans': > > fs/compat_ioctl.c:859: warning: 'fptr32' might be used uninitialized in this function > > fs/compat_ioctl.c: In function `do_fontx_ioctl': > > fs/compat_ioctl.c:1082: warning: 'data' might be used uninitialized in this function > > fs/compat_ioctl.c: In function `do_atmif_sioc': > > fs/compat_ioctl.c:1302: warning: 'data' might be used uninitialized in this function > > fs/compat_ioctl.c: In function `do_atm_ioctl': > > fs/compat_ioctl.c:1272: warning: 'data' might be used uninitialized in this function > > fs/compat_ioctl.c: In function `mtd_rw_oob': > > fs/compat_ioctl.c:1421: warning: 'data' might be used uninitialized in this function > > fs/compat_ioctl.c: In function `do_usbdevfs_discsignal': > > fs/compat_ioctl.c:1650: warning: 'uctx' might be used uninitialized in this function > > > > This is: > > if (get_user(flen, &u_fprog32->len) || > > get_user(fptr32, &u_fprog32->filter)) > > return -EFAULT; > > Is this all sparc cross compiler? I'm trying to reproduce it on x86 with > no avail :-( > > I would like to test other ways to change the macro, but to do so, I need > to get a compiler that will produce the warnings that you see. What > version of gcc are you using? I used crosstool to build a 3.4.5 gcc: set -ex TARBALLS_DIR=$HOME/downloads RESULT_TOP=/opt/crosstool export TARBALLS_DIR RESULT_TOP GCC_LANGUAGES="c" export GCC_LANGUAGES eval `cat sparc64.dat gcc-3.4.5-glibc-2.3.6-tls.dat` sh all.sh --notest It is running on a 32 bit target. processor : 0 vendor_id : GenuineIntel cpu family : 6 model : 8 model name : Pentium III (Coppermine) stepping : 10 cpu MHz : 851.970 cache size : 256 KB fdiv_bug : no hlt_bug : no f00f_bug : no coma_bug : no fpu : yes fpu_exception : yes cpuid level : 2 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 mtrr pge mca cmov pat pse36 mmx fxsr sse bogomips : 1704.50 clflush size : 32 Sam ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 22:01 ` Sam Ravnborg @ 2009-01-05 22:14 ` Steven Rostedt 2009-01-05 23:11 ` Heiko Carstens 0 siblings, 1 reply; 37+ messages in thread From: Steven Rostedt @ 2009-01-05 22:14 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, 5 Jan 2009, Sam Ravnborg wrote: > > Is this all sparc cross compiler? I'm trying to reproduce it on x86 with > > no avail :-( > > > > I would like to test other ways to change the macro, but to do so, I need > > to get a compiler that will produce the warnings that you see. What > > version of gcc are you using? > > I used crosstool to build a 3.4.5 gcc: Hmm, that's a pretty old compiler. I wonder if it wouldn't just help if we just make the branch tracer dependent on the compiler used. That is. #if defined(CONFIG_PROFILE_ALL_BRANCHES) && (__GNUC__ >= 4) #define if(cond) ... Or something :-/ -- Steve ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 22:14 ` Steven Rostedt @ 2009-01-05 23:11 ` Heiko Carstens 2009-01-06 2:07 ` Steven Rostedt 2009-01-06 4:30 ` Steven Rostedt 0 siblings, 2 replies; 37+ messages in thread From: Heiko Carstens @ 2009-01-05 23:11 UTC (permalink / raw) To: Steven Rostedt Cc: Sam Ravnborg, LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, Jan 05, 2009 at 05:14:22PM -0500, Steven Rostedt wrote: > > On Mon, 5 Jan 2009, Sam Ravnborg wrote: > > > > Is this all sparc cross compiler? I'm trying to reproduce it on x86 with > > > no avail :-( > > > > > > I would like to test other ways to change the macro, but to do so, I need > > > to get a compiler that will produce the warnings that you see. What > > > version of gcc are you using? > > > > I used crosstool to build a 3.4.5 gcc: > > Hmm, that's a pretty old compiler. I wonder if it wouldn't just help > if we just make the branch tracer dependent on the compiler used. That is. > > #if defined(CONFIG_PROFILE_ALL_BRANCHES) && (__GNUC__ >= 4) > #define if(cond) ... > > Or something :-/ FWIW, on s390 with gcc 4.3.2 and an allyesconfig I get these: CC arch/s390/mm/extmem.o arch/s390/mm/extmem.c: In function 'segment_modify_shared': arch/s390/mm/extmem.c:572: warning: 'end_addr' may be used uninitialized in this function arch/s390/mm/extmem.c:572: warning: 'start_addr' may be used uninitialized in this function arch/s390/mm/extmem.c: In function 'query_segment_type': arch/s390/mm/extmem.c:259: warning: 'vmrc' may be used uninitialized in this function Switching off PROFILE_ALL_BRANCHES makes the warnings go away again. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 23:11 ` Heiko Carstens @ 2009-01-06 2:07 ` Steven Rostedt 2009-01-06 9:36 ` Heiko Carstens 2009-01-06 4:30 ` Steven Rostedt 1 sibling, 1 reply; 37+ messages in thread From: Steven Rostedt @ 2009-01-06 2:07 UTC (permalink / raw) To: Heiko Carstens Cc: Sam Ravnborg, LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Tue, 6 Jan 2009, Heiko Carstens wrote: > On Mon, Jan 05, 2009 at 05:14:22PM -0500, Steven Rostedt wrote: > > > > On Mon, 5 Jan 2009, Sam Ravnborg wrote: > > > > > > Is this all sparc cross compiler? I'm trying to reproduce it on x86 with > > > > no avail :-( > > > > > > > > I would like to test other ways to change the macro, but to do so, I need > > > > to get a compiler that will produce the warnings that you see. What > > > > version of gcc are you using? > > > > > > I used crosstool to build a 3.4.5 gcc: > > > > Hmm, that's a pretty old compiler. I wonder if it wouldn't just help > > if we just make the branch tracer dependent on the compiler used. That is. > > > > #if defined(CONFIG_PROFILE_ALL_BRANCHES) && (__GNUC__ >= 4) > > #define if(cond) ... > > > > Or something :-/ > > FWIW, on s390 with gcc 4.3.2 and an allyesconfig I get these: > > CC arch/s390/mm/extmem.o > arch/s390/mm/extmem.c: In function 'segment_modify_shared': > arch/s390/mm/extmem.c:572: warning: 'end_addr' may be used uninitialized in this function > arch/s390/mm/extmem.c:572: warning: 'start_addr' may be used uninitialized in this function > arch/s390/mm/extmem.c: In function 'query_segment_type': > arch/s390/mm/extmem.c:259: warning: 'vmrc' may be used uninitialized in this function > > Switching off PROFILE_ALL_BRANCHES makes the warnings go away again. Now that is really interesting. Because end_addr and start_addr are initialized via functions: if (x) init_me(a, &y); else init_me(b, &y); Which actually does not make sense why turning off PROFILE_ALL_BRANCHES would affect this :-/ -- Steve ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 2:07 ` Steven Rostedt @ 2009-01-06 9:36 ` Heiko Carstens 0 siblings, 0 replies; 37+ messages in thread From: Heiko Carstens @ 2009-01-06 9:36 UTC (permalink / raw) To: Steven Rostedt Cc: Sam Ravnborg, LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, Jan 05, 2009 at 09:07:05PM -0500, Steven Rostedt wrote: > On Tue, 6 Jan 2009, Heiko Carstens wrote: > > On Mon, Jan 05, 2009 at 05:14:22PM -0500, Steven Rostedt wrote: > > > On Mon, 5 Jan 2009, Sam Ravnborg wrote: > > > > > > > > Is this all sparc cross compiler? I'm trying to reproduce it on x86 with > > > > > no avail :-( > > > > > > > > > > I would like to test other ways to change the macro, but to do so, I need > > > > > to get a compiler that will produce the warnings that you see. What > > > > > version of gcc are you using? > > > > > > > > I used crosstool to build a 3.4.5 gcc: > > > > > > Hmm, that's a pretty old compiler. I wonder if it wouldn't just help > > > if we just make the branch tracer dependent on the compiler used. That is. > > > > > > #if defined(CONFIG_PROFILE_ALL_BRANCHES) && (__GNUC__ >= 4) > > > #define if(cond) ... > > > > > > Or something :-/ > > > > FWIW, on s390 with gcc 4.3.2 and an allyesconfig I get these: > > > > CC arch/s390/mm/extmem.o > > arch/s390/mm/extmem.c: In function 'segment_modify_shared': > > arch/s390/mm/extmem.c:572: warning: 'end_addr' may be used uninitialized in this function > > arch/s390/mm/extmem.c:572: warning: 'start_addr' may be used uninitialized in this function > > arch/s390/mm/extmem.c: In function 'query_segment_type': > > arch/s390/mm/extmem.c:259: warning: 'vmrc' may be used uninitialized in this function > > > > Switching off PROFILE_ALL_BRANCHES makes the warnings go away again. > > Now that is really interesting. Because end_addr and start_addr are > initialized via functions: > > if (x) > init_me(a, &y); > else > init_me(b, &y); > > Which actually does not make sense why turning off PROFILE_ALL_BRANCHES > would affect this :-/ They are not necessarily initialized via 'initme'. Only if it returns with a return value >= 0. So it's a bit more complex: if (x) rc = init_me(a, &y); else rc = init_me(b, &y); if (rc < 0) /* y unitialized */ return; use(y); ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 23:11 ` Heiko Carstens 2009-01-06 2:07 ` Steven Rostedt @ 2009-01-06 4:30 ` Steven Rostedt 2009-01-06 9:45 ` Heiko Carstens 1 sibling, 1 reply; 37+ messages in thread From: Steven Rostedt @ 2009-01-06 4:30 UTC (permalink / raw) To: Heiko Carstens Cc: Sam Ravnborg, LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Tue, 6 Jan 2009, Heiko Carstens wrote: Sam and Heiko, I'm trying to see if the (a ? b : c) construct is causing the issue. Can you test this patch. Thanks, -- Steve diff --git a/include/linux/compiler.h b/include/linux/compiler.h index d95da10..e13ad24 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -113,7 +113,8 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect); * "Define 'is'", Bill Clinton * "Define 'if'", Steven Rostedt */ -#define if(cond) if (__builtin_constant_p((cond)) ? !!(cond) : \ +#define if(cond) if ((__builtin_constant_p((cond)) && !!(cond)) || \ + (!__builtin_constant_p((cond)) && \ ({ \ int ______r; \ static struct ftrace_branch_data \ @@ -130,7 +131,7 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect); else \ ______f.miss++; \ ______r; \ - })) + }))) #endif /* CONFIG_PROFILE_ALL_BRANCHES */ #else ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 4:30 ` Steven Rostedt @ 2009-01-06 9:45 ` Heiko Carstens 0 siblings, 0 replies; 37+ messages in thread From: Heiko Carstens @ 2009-01-06 9:45 UTC (permalink / raw) To: Steven Rostedt Cc: Sam Ravnborg, LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux On Mon, Jan 05, 2009 at 11:30:58PM -0500, Steven Rostedt wrote: > On Tue, 6 Jan 2009, Heiko Carstens wrote: > Sam and Heiko, > > I'm trying to see if the (a ? b : c) construct is causing the issue. Can > you test this patch. > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index d95da10..e13ad24 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -113,7 +113,8 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect); > * "Define 'is'", Bill Clinton > * "Define 'if'", Steven Rostedt > */ > -#define if(cond) if (__builtin_constant_p((cond)) ? !!(cond) : \ > +#define if(cond) if ((__builtin_constant_p((cond)) && !!(cond)) || \ > + (!__builtin_constant_p((cond)) && \ Doesn't help unfortunately. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 20:05 ` Steven Rostedt 2009-01-05 21:31 ` Sam Ravnborg @ 2009-01-06 18:32 ` David Miller 2009-01-06 18:52 ` Steven Rostedt 1 sibling, 1 reply; 37+ messages in thread From: David Miller @ 2009-01-06 18:32 UTC (permalink / raw) To: rostedt; +Cc: sam, linux-kernel, srostedt, mingo, sparclinux From: Steven Rostedt <rostedt@goodmis.org> Date: Mon, 5 Jan 2009 15:05:11 -0500 (EST) > Probably the same issue. The problem is that the first use of a variable > is in the OR section of an if statement that does a return. > > if (x || !(y = init_me()) > return; > > use_me(y); > > IMHO I find this sloppy code. When reading the code it can cause reviewers > trouble, and wasted time, to see that y is indeed initialized. I'm > impressed that gcc was able to figure it out. I'm pretty much in firm disagreement here. This code is pretty clear. The only way to get to use_me() is by initializing 'y'. It's very straightforward. There are many conditionals in the kernel where this order of evaluation and side effects is depended upon. Some of them just happen to warn now because of the branch tracer. > Have you always been compiling with -Werror? arch/sparc*/ builds with -Werror for 5+ years. > The reason that gcc complains is because you have the > "branch_tracer" on that converts 'if ()' into a macro (as you saw in > your -E compile). This makes the if statement more complex, and goes > beyond gcc's ability to know that the above 'y' is initialized > properly. I would work on fixing this in the branch tracer, but > honestly, I'm kind of glad that gcc barfs on it. This will help us > point out this kind of sloppy initializations (sorry if I'm > offending anybody about calling it sloppy). I just believe that it > makes the code a bit more obfuscated to initialize in an if > statement, and a second part of a complex if statement at that! Keep in mind all this code was fine and built warning free before the if() obfuscation done by the branch tracer. If I wrote the branch tracer, I'd probably search for these kinds of excuses too :-) ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 18:32 ` David Miller @ 2009-01-06 18:52 ` Steven Rostedt 2009-01-06 19:01 ` David Miller 0 siblings, 1 reply; 37+ messages in thread From: Steven Rostedt @ 2009-01-06 18:52 UTC (permalink / raw) To: David Miller; +Cc: sam, linux-kernel, srostedt, mingo, sparclinux On Tue, 6 Jan 2009, David Miller wrote: > From: Steven Rostedt <rostedt@goodmis.org> > Date: Mon, 5 Jan 2009 15:05:11 -0500 (EST) > > > Probably the same issue. The problem is that the first use of a variable > > is in the OR section of an if statement that does a return. > > > > if (x || !(y = init_me()) > > return; > > > > use_me(y); > > > > IMHO I find this sloppy code. When reading the code it can cause reviewers > > trouble, and wasted time, to see that y is indeed initialized. I'm > > impressed that gcc was able to figure it out. > > I'm pretty much in firm disagreement here. fair enough. > > This code is pretty clear. The only way to get to use_me() is > by initializing 'y'. It's very straightforward. > > There are many conditionals in the kernel where this order > of evaluation and side effects is depended upon. Some of them > just happen to warn now because of the branch tracer. > > > Have you always been compiling with -Werror? > > arch/sparc*/ builds with -Werror for 5+ years. > > > The reason that gcc complains is because you have the > > "branch_tracer" on that converts 'if ()' into a macro (as you saw in > > your -E compile). This makes the if statement more complex, and goes > > beyond gcc's ability to know that the above 'y' is initialized > > properly. I would work on fixing this in the branch tracer, but > > honestly, I'm kind of glad that gcc barfs on it. This will help us > > point out this kind of sloppy initializations (sorry if I'm > > offending anybody about calling it sloppy). I just believe that it > > makes the code a bit more obfuscated to initialize in an if > > statement, and a second part of a complex if statement at that! > > Keep in mind all this code was fine and built warning free before the > if() obfuscation done by the branch tracer. If I wrote the branch > tracer, I'd probably search for these kinds of excuses too :-) Yeah, I'm trying different ways to handle the if magic to see if I can come up with a way to keep gcc happy. Unfortunately, every solution so far still seems to confuse sparc (I don't know why it does not confuse x86 or powerpc, although some of my variations do). I would hate to black list archs just because it gives warnings. I've analyzed the code greatly to make sure that it keeps the semantics of the original code the same. It's just that it pushes gcc beyond its threshold of knowing that a variable is initialized. I do not blame gcc because it has to have a limit to handle logic cases (otherwise it has solved the halting problem). What bothers me is that some versions of gcc are not affected by the if macro and some are. I'm impressed to hear that sparc has been compiling fine for years with -Werror since I'm not sure other archs can say the same. I'll have to keep hacking at it. :-/ -- Steve ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 18:52 ` Steven Rostedt @ 2009-01-06 19:01 ` David Miller 2009-01-06 19:52 ` Sam Ravnborg 0 siblings, 1 reply; 37+ messages in thread From: David Miller @ 2009-01-06 19:01 UTC (permalink / raw) To: rostedt; +Cc: sam, linux-kernel, srostedt, mingo, sparclinux From: Steven Rostedt <rostedt@goodmis.org> Date: Tue, 6 Jan 2009 13:52:36 -0500 (EST) > I would hate to black list archs just because it gives warnings. At the very least arch/sparc should build cleanly now because I took your ldc.c change ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 19:01 ` David Miller @ 2009-01-06 19:52 ` Sam Ravnborg 2009-01-06 20:02 ` David Miller 0 siblings, 1 reply; 37+ messages in thread From: Sam Ravnborg @ 2009-01-06 19:52 UTC (permalink / raw) To: David Miller; +Cc: rostedt, linux-kernel, srostedt, mingo, sparclinux On Tue, Jan 06, 2009 at 11:01:14AM -0800, David Miller wrote: > From: Steven Rostedt <rostedt@goodmis.org> > Date: Tue, 6 Jan 2009 13:52:36 -0500 (EST) > > > I would hate to black list archs just because it gives warnings. > > At the very least arch/sparc should build cleanly now because > I took your ldc.c change Unfortunately not. With sparc64 I saw warnings in three additional files. I cooked up the following to avoid the warnings. The patch to unaligned_64.c is just an ugly hack. The patch in viohs.c makes it easier to read IMO. The patch to init_64.c is likewise a nice cleanup. I will re-review and submit the latter two as proper patches. I see no way to refactor unaligned_64.c so it keep current behaviour and is equal readable. Sam diff --git a/arch/sparc/kernel/unaligned_64.c b/arch/sparc/kernel/unaligned_64.c index 203ddfa..f005799 100644 --- a/arch/sparc/kernel/unaligned_64.c +++ b/arch/sparc/kernel/unaligned_64.c @@ -601,11 +601,13 @@ void handle_lddfmna(struct pt_regs *regs, unsigned long sfar, unsigned long sfsr pc = (u32)pc; if (get_user(insn, (u32 __user *) pc) != -EFAULT) { int asi = decode_asi(insn, regs); + int rfirst, rsecond; if ((asi > ASI_SNFL) || (asi < ASI_P)) goto daex; - if (get_user(first, (u32 __user *)sfar) || - get_user(second, (u32 __user *)(sfar + 4))) { + rfirst = get_user(first, (u32 __user *)sfar); + rsecond = get_user(second, (u32 __user *)(sfar + 4)); + if (rfirst || rsecond) { if (asi & 0x2) /* NF */ { first = 0; second = 0; } else diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c index 708fa17..aa6ac70 100644 --- a/arch/sparc/kernel/viohs.c +++ b/arch/sparc/kernel/viohs.c @@ -337,8 +337,10 @@ static int process_ver_nack(struct vio_driver_state *vio, viodbg(HS, "GOT VERSION NACK maj[%u] min[%u] devclass[%u]\n", pkt->major, pkt->minor, pkt->dev_class); - if ((pkt->major == 0 && pkt->minor == 0) || - !(nver = find_by_major(vio, pkt->major))) + if (pkt->major == 0 && pkt->minor == 0) + return handshake_failure(vio); + nver = find_by_major(vio, pkt->major); + if (!nver) return handshake_failure(vio); if (send_version(vio, nver->major, nver->minor) < 0) diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c index c04b111..26f59df 100644 --- a/arch/sparc/mm/init_64.c +++ b/arch/sparc/mm/init_64.c @@ -259,21 +259,15 @@ static inline void tsb_insert(struct tsb *ent, unsigned long tag, unsigned long unsigned long _PAGE_ALL_SZ_BITS __read_mostly; unsigned long _PAGE_SZBITS __read_mostly; -void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t pte) +static void try_flush_dcache(unsigned long pfn) { - struct mm_struct *mm; - struct tsb *tsb; - unsigned long tag, flags; - unsigned long tsb_index, tsb_hash_shift; + unsigned long pg_flags; + struct page *page; - if (tlb_type != hypervisor) { - unsigned long pfn = pte_pfn(pte); - unsigned long pg_flags; - struct page *page; - - if (pfn_valid(pfn) && - (page = pfn_to_page(pfn), page_mapping(page)) && - ((pg_flags = page->flags) & (1UL << PG_dcache_dirty))) { + page = pfn_to_page(pfn); + if (page && page_mapping(page)) { + pg_flags = page->flags; + if (pg_flags & (1UL << PG_dcache_dirty)) { int cpu = ((pg_flags >> PG_dcache_cpu_shift) & PG_dcache_cpu_mask); int this_cpu = get_cpu(); @@ -291,6 +285,21 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t p put_cpu(); } } +} + +void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t pte) +{ + struct mm_struct *mm; + struct tsb *tsb; + unsigned long tag, flags; + unsigned long tsb_index, tsb_hash_shift; + + if (tlb_type != hypervisor) { + unsigned long pfn = pte_pfn(pte); + + if (pfn_valid(pfn)) + try_flush_dcache(pfn); + } mm = vma->vm_mm; ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 19:52 ` Sam Ravnborg @ 2009-01-06 20:02 ` David Miller 0 siblings, 0 replies; 37+ messages in thread From: David Miller @ 2009-01-06 20:02 UTC (permalink / raw) To: sam; +Cc: rostedt, linux-kernel, srostedt, mingo, sparclinux From: Sam Ravnborg <sam@ravnborg.org> Date: Tue, 6 Jan 2009 20:52:06 +0100 > The patch to unaligned_64.c is just an ugly hack. > The patch in viohs.c makes it easier to read IMO. > The patch to init_64.c is likewise a nice cleanup. > > I will re-review and submit the latter two as proper patches. > I see no way to refactor unaligned_64.c so it keep > current behaviour and is equal readable. Ok. ^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH] module: clean up initialization of variable 2009-01-05 19:54 ` ftrace breaks sparc64 build Sam Ravnborg 2009-01-05 20:05 ` Steven Rostedt @ 2009-01-05 20:30 ` Steven Rostedt 2009-01-05 22:59 ` Harvey Harrison 2009-01-06 1:22 ` Rusty Russell 1 sibling, 2 replies; 37+ messages in thread From: Steven Rostedt @ 2009-01-05 20:30 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux, Rusty Russell, Andrew Morton Impact: clean up On Mon, 5 Jan 2009, Sam Ravnborg wrote: > > Fully ageed on the readability. > I happen to trigger this as an error in the sparc code. > But I see the same warning also in generic code. > > >From kernel/module.c: > /* Suck in entire file: we'll want most of it. */ > /* vmalloc barfs on "unusual" numbers. Check here */ > if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL) > return ERR_PTR(-ENOMEM); > > > This gives following warning: > kernel/module.c: In function `load_module': > kernel/module.c:1842: warning: 'hdr' might be used uninitialized in this function The above is caused by having the branch tracer on and the following type of initialization: if (x || !(y = init_me()) return; use_me(y); This is sloppy initialization because it initializes, not only in an if condition, but also as the second part of a complex conditional. This patch makes the code a bit easier to read. Signed-off-by: Steven Rostedt <srostedt@redhat.com> Reported-by: Sam Ravnborg <sam@ravnborg.org> diff --git a/kernel/module.c b/kernel/module.c index 9712c52..112d6cd 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1864,7 +1864,10 @@ static noinline struct module *load_module(void __user *umod, /* Suck in entire file: we'll want most of it. */ /* vmalloc barfs on "unusual" numbers. Check here */ - if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL) + if (len > 64 * 1024 * 1024) + return ERR_PTR(-ENOMEM); + hdr = vmalloc(len); + if (hdr == NULL) return ERR_PTR(-ENOMEM); if (copy_from_user(hdr, umod, len) != 0) { err = -EFAULT; ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH] module: clean up initialization of variable 2009-01-05 20:30 ` [PATCH] module: clean up initialization of variable Steven Rostedt @ 2009-01-05 22:59 ` Harvey Harrison 2009-01-06 1:22 ` Rusty Russell 1 sibling, 0 replies; 37+ messages in thread From: Harvey Harrison @ 2009-01-05 22:59 UTC (permalink / raw) To: Steven Rostedt Cc: Sam Ravnborg, LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux, Rusty Russell, Andrew Morton On Mon, 2009-01-05 at 15:30 -0500, Steven Rostedt wrote: > + hdr = vmalloc(len); > + if (hdr == NULL) A bit pedantic but, if (!hdr) perhaps? Cheers, Harvey ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] module: clean up initialization of variable 2009-01-05 20:30 ` [PATCH] module: clean up initialization of variable Steven Rostedt 2009-01-05 22:59 ` Harvey Harrison @ 2009-01-06 1:22 ` Rusty Russell 2009-01-06 2:02 ` Steven Rostedt 1 sibling, 1 reply; 37+ messages in thread From: Rusty Russell @ 2009-01-06 1:22 UTC (permalink / raw) To: Steven Rostedt Cc: Sam Ravnborg, LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux, Andrew Morton On Tuesday 06 January 2009 07:00:25 Steven Rostedt wrote: > This is sloppy initialization because it initializes, not only in an > if condition, but also as the second part of a complex conditional. > > This patch makes the code a bit easier to read. ... > /* Suck in entire file: we'll want most of it. */ > /* vmalloc barfs on "unusual" numbers. Check here */ > - if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL) > + if (len > 64 * 1024 * 1024) > + return ERR_PTR(-ENOMEM); > + hdr = vmalloc(len); > + if (hdr == NULL) > return ERR_PTR(-ENOMEM); > if (copy_from_user(hdr, umod, len) != 0) { > err = -EFAULT; This line is not accidental nor casually written: the two statements are deliberately entwined. It is a succint complaint against the vagaries of vmalloc. So this patch is a messup, not a cleanup. But it's really upset me because it is lazy and timid: and too much kernel code is becoming mired in such scars. Instead of "how do I kill this warning and get it in the merge window" you should be thinking "how do I make the kernel better", and "I wonder if vmalloc still has this problem"... And I so look forward to the warm fuzzies I get when applying a real cleanup patch. Thanks, Rusty. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] module: clean up initialization of variable 2009-01-06 1:22 ` Rusty Russell @ 2009-01-06 2:02 ` Steven Rostedt 0 siblings, 0 replies; 37+ messages in thread From: Steven Rostedt @ 2009-01-06 2:02 UTC (permalink / raw) To: Rusty Russell Cc: Sam Ravnborg, LKML, Steven Rostedt, Ingo Molnar, David S. Miller, sparclinux, Andrew Morton On Tue, 6 Jan 2009, Rusty Russell wrote: > On Tuesday 06 January 2009 07:00:25 Steven Rostedt wrote: > > This is sloppy initialization because it initializes, not only in an > > if condition, but also as the second part of a complex conditional. > > > > This patch makes the code a bit easier to read. > ... > > /* Suck in entire file: we'll want most of it. */ > > /* vmalloc barfs on "unusual" numbers. Check here */ > > - if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL) > > + if (len > 64 * 1024 * 1024) > > + return ERR_PTR(-ENOMEM); > > + hdr = vmalloc(len); > > + if (hdr == NULL) > > return ERR_PTR(-ENOMEM); > > if (copy_from_user(hdr, umod, len) != 0) { > > err = -EFAULT; > > This line is not accidental nor casually written: the two statements > are deliberately entwined. It is a succint complaint against the > vagaries of vmalloc. > > So this patch is a messup, not a cleanup. It is not that much of a messup. I did not realize that the code was a political protest against the horrors of vmalloc ;-) > > But it's really upset me because it is lazy and timid: and too much > kernel code is becoming mired in such scars. Instead of "how do I kill > this warning and get it in the merge window" you should be thinking "how > do I make the kernel better", and "I wonder if vmalloc still has this > problem"... > > And I so look forward to the warm fuzzies I get when applying a real > cleanup patch. Well, I'm not about to go solve the vmalloc issues (not today anyway). But I'll go and see if I can get the branch tracer macro to work on all versions of gcc. Thanks, -- Steve ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 18:19 ftrace breaks sparc64 build Sam Ravnborg 2009-01-05 19:31 ` Steven Rostedt @ 2009-01-05 19:48 ` Al Viro 2009-01-05 19:55 ` Sam Ravnborg 1 sibling, 1 reply; 37+ messages in thread From: Al Viro @ 2009-01-05 19:48 UTC (permalink / raw) To: Sam Ravnborg Cc: LKML, Steven Rostedt, Ingo Molnar, rostedt, David S. Miller, sparclinux On Mon, Jan 05, 2009 at 07:19:22PM +0100, Sam Ravnborg wrote: > With an allmodconfig build on sparc and sparc64 I started > to see warnings that become propagated to errors by -Werror. While we are at it, sparc32 allmodconfig is broken by commit 9bb482476c6c9d1ae033306440c51ceac93ea80c Author: Jan Beulich <jbeulich@novell.com> Date: Tue Dec 16 11:30:08 2008 +0000 allow stripping of generated symbols under CONFIG_KALLSYMS_ALL Results in sparc-linux-objcopy: not stripping symbol `__ksymtab_strings' because it is named in a relocation sparc-linux-objcopy: not stripping symbol `__crc_per_cpu__softirq_work_list' because it is named in a relocation sparc-linux-objcopy: not stripping symbol `__initcall_end' because it is named in a relocation sparc-linux-objcopy: not stripping symbol `__setup_end' because it is named in a relocation sparc-linux-objcopy: not stripping symbol `__setup_start' because it is named in a relocation sparc-linux-objcopy: not stripping symbol `__initcall_start' because it is named in a relocation make: *** [.tmp_vmlinux1.stripped] Error 1 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 19:48 ` ftrace breaks sparc64 build Al Viro @ 2009-01-05 19:55 ` Sam Ravnborg 2009-01-06 7:53 ` Jan Beulich 0 siblings, 1 reply; 37+ messages in thread From: Sam Ravnborg @ 2009-01-05 19:55 UTC (permalink / raw) To: Al Viro, Jan Beulich Cc: LKML, Steven Rostedt, Ingo Molnar, rostedt, David S. Miller, sparclinux Added Jan. And yes - I see the same but with a slightly different error message. Sam On Mon, Jan 05, 2009 at 07:48:44PM +0000, Al Viro wrote: > On Mon, Jan 05, 2009 at 07:19:22PM +0100, Sam Ravnborg wrote: > > With an allmodconfig build on sparc and sparc64 I started > > to see warnings that become propagated to errors by -Werror. > > While we are at it, sparc32 allmodconfig is broken by > commit 9bb482476c6c9d1ae033306440c51ceac93ea80c > Author: Jan Beulich <jbeulich@novell.com> > Date: Tue Dec 16 11:30:08 2008 +0000 > > allow stripping of generated symbols under CONFIG_KALLSYMS_ALL > > Results in > sparc-linux-objcopy: not stripping symbol `__ksymtab_strings' because it is named in a relocation > sparc-linux-objcopy: not stripping symbol `__crc_per_cpu__softirq_work_list' because it is named in a relocation > sparc-linux-objcopy: not stripping symbol `__initcall_end' because it is named in a relocation > sparc-linux-objcopy: not stripping symbol `__setup_end' because it is named in a relocation > sparc-linux-objcopy: not stripping symbol `__setup_start' because it is named in a relocation > sparc-linux-objcopy: not stripping symbol `__initcall_start' because it is named in a relocation > make: *** [.tmp_vmlinux1.stripped] Error 1 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-05 19:55 ` Sam Ravnborg @ 2009-01-06 7:53 ` Jan Beulich 2009-01-06 11:35 ` Al Viro 0 siblings, 1 reply; 37+ messages in thread From: Jan Beulich @ 2009-01-06 7:53 UTC (permalink / raw) To: Sam Ravnborg, Al Viro Cc: David S. Miller, Ingo Molnar, rostedt, Steven Rostedt, LKML, sparclinux >On Mon, Jan 05, 2009 at 07:48:44PM +0000, Al Viro wrote: >> On Mon, Jan 05, 2009 at 07:19:22PM +0100, Sam Ravnborg wrote: >> > With an allmodconfig build on sparc and sparc64 I started >> > to see warnings that become propagated to errors by -Werror. >> >> While we are at it, sparc32 allmodconfig is broken by >> commit 9bb482476c6c9d1ae033306440c51ceac93ea80c >> Author: Jan Beulich <jbeulich@novell.com> >> Date: Tue Dec 16 11:30:08 2008 +0000 >> >> allow stripping of generated symbols under CONFIG_KALLSYMS_ALL >> >> Results in >> sparc-linux-objcopy: not stripping symbol `__ksymtab_strings' because it is named in a relocation >> sparc-linux-objcopy: not stripping symbol `__crc_per_cpu__softirq_work_list' because it is named in a relocation >> sparc-linux-objcopy: not stripping symbol `__initcall_end' because it is named in a relocation >> sparc-linux-objcopy: not stripping symbol `__setup_end' because it is named in a relocation >> sparc-linux-objcopy: not stripping symbol `__setup_start' because it is named in a relocation >> sparc-linux-objcopy: not stripping symbol `__initcall_start' because it is named in a relocation >> make: *** [.tmp_vmlinux1.stripped] Error 1 The __crc_... reference is definitely bogus - none should survive with the new .c->.o rule. Could you find out what object file they originate from? The others look like a tools side behavioral difference, as I never saw any such. Is this problem sparc32-specific (I tested x86 and ia64 only)? What's the binutils version used? Jan ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 7:53 ` Jan Beulich @ 2009-01-06 11:35 ` Al Viro 2009-01-06 12:39 ` Jan Beulich ` (2 more replies) 0 siblings, 3 replies; 37+ messages in thread From: Al Viro @ 2009-01-06 11:35 UTC (permalink / raw) To: Jan Beulich Cc: Sam Ravnborg, David S. Miller, Ingo Molnar, rostedt, Steven Rostedt, LKML, sparclinux On Tue, Jan 06, 2009 at 07:53:04AM +0000, Jan Beulich wrote: > The __crc_... reference is definitely bogus - none should survive with the > new .c->.o rule. Could you find out what object file they originate from? So can you, by use of arcane tool known as "grep"... It's in kernel/softirq.c and that's genksyms parser being fucked in head. Look for TYPEOF_KEYW in parse.y and you'll see. Especially amusing part is a kludge from commit a89a0a2354ae666612968e254d650bfd04f11eb6... > The others look like a tools side behavioral difference, as I never saw any > such. Is this problem sparc32-specific (I tested x86 and ia64 only)? What's > the binutils version used? 2.18.50.0.6. And no, it's not tools side. What it is, AFAICT, is that sparc32 has LDFLAGS_vmlinux = -r, which leaves a metric arseload of relocs that wouldn't have survived into vmlinux otherwise. Look at .rela__ksymtab in .tmp_vmlinux1, for example... ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 11:35 ` Al Viro @ 2009-01-06 12:39 ` Jan Beulich 2009-01-06 13:34 ` Sam Ravnborg 2009-01-08 9:28 ` Jan Beulich 2 siblings, 0 replies; 37+ messages in thread From: Jan Beulich @ 2009-01-06 12:39 UTC (permalink / raw) To: Sam Ravnborg, Al Viro Cc: David S. Miller, Ingo Molnar, rostedt, Steven Rostedt, LKML, sparclinux >>> Al Viro <viro@ZenIV.linux.org.uk> 06.01.09 12:35 >>> >On Tue, Jan 06, 2009 at 07:53:04AM +0000, Jan Beulich wrote: >> The others look like a tools side behavioral difference, as I never saw any >> such. Is this problem sparc32-specific (I tested x86 and ia64 only)? What's >> the binutils version used? > >2.18.50.0.6. > >And no, it's not tools side. What it is, AFAICT, is that sparc32 has >LDFLAGS_vmlinux = -r, which leaves a metric arseload of relocs that >wouldn't have survived into vmlinux otherwise. Look at .rela__ksymtab >in .tmp_vmlinux1, for example... That I think can only be taken care of by disallowing the stripping for that arch, i.e. adding a negative dependency to the Kconfig option. Or can anyone think of any other solution? Jan ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 11:35 ` Al Viro 2009-01-06 12:39 ` Jan Beulich @ 2009-01-06 13:34 ` Sam Ravnborg 2009-01-06 15:52 ` Al Viro 2009-01-06 18:39 ` David Miller 2009-01-08 9:28 ` Jan Beulich 2 siblings, 2 replies; 37+ messages in thread From: Sam Ravnborg @ 2009-01-06 13:34 UTC (permalink / raw) To: Al Viro Cc: Jan Beulich, David S. Miller, Ingo Molnar, rostedt, Steven Rostedt, LKML, sparclinux On Tue, Jan 06, 2009 at 11:35:43AM +0000, Al Viro wrote: > On Tue, Jan 06, 2009 at 07:53:04AM +0000, Jan Beulich wrote: > > > The __crc_... reference is definitely bogus - none should survive with the > > new .c->.o rule. Could you find out what object file they originate from? > > So can you, by use of arcane tool known as "grep"... It's in kernel/softirq.c > and that's genksyms parser being fucked in head. Look for TYPEOF_KEYW > in parse.y and you'll see. Especially amusing part is a kludge from > commit a89a0a2354ae666612968e254d650bfd04f11eb6... Any feedback on what to do to make it better? Never used typeof myself so I do not know the exact syntax. > > > The others look like a tools side behavioral difference, as I never saw any > > such. Is this problem sparc32-specific (I tested x86 and ia64 only)? What's > > the binutils version used? > > 2.18.50.0.6. > > And no, it's not tools side. What it is, AFAICT, is that sparc32 has > LDFLAGS_vmlinux = -r, which leaves a metric arseload of relocs that > wouldn't have survived into vmlinux otherwise. Look at .rela__ksymtab > in .tmp_vmlinux1, for example... The use of -r is to support the btfixup magic. I have been thinking of moving the btfixup phase so it happens _before_ the final link of vmlinux. This should help us here. But I never managed to really understand what the btfixup thing is all about and has then been sidetracked by funnier stuff. Sam ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 13:34 ` Sam Ravnborg @ 2009-01-06 15:52 ` Al Viro 2009-01-06 18:39 ` David Miller 1 sibling, 0 replies; 37+ messages in thread From: Al Viro @ 2009-01-06 15:52 UTC (permalink / raw) To: Sam Ravnborg Cc: Jan Beulich, David S. Miller, Ingo Molnar, rostedt, Steven Rostedt, LKML, sparclinux On Tue, Jan 06, 2009 at 02:34:42PM +0100, Sam Ravnborg wrote: > On Tue, Jan 06, 2009 at 11:35:43AM +0000, Al Viro wrote: > > On Tue, Jan 06, 2009 at 07:53:04AM +0000, Jan Beulich wrote: > > > > > The __crc_... reference is definitely bogus - none should survive with the > > > new .c->.o rule. Could you find out what object file they originate from? > > > > So can you, by use of arcane tool known as "grep"... It's in kernel/softirq.c > > and that's genksyms parser being fucked in head. Look for TYPEOF_KEYW > > in parse.y and you'll see. Especially amusing part is a kludge from > > commit a89a0a2354ae666612968e254d650bfd04f11eb6... > > Any feedback on what to do to make it better? > Never used typeof myself so I do not know the exact syntax. Same as that of sizeof. I.e. typeof(<type-name>) or typeof <unary-expression>. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 13:34 ` Sam Ravnborg 2009-01-06 15:52 ` Al Viro @ 2009-01-06 18:39 ` David Miller 1 sibling, 0 replies; 37+ messages in thread From: David Miller @ 2009-01-06 18:39 UTC (permalink / raw) To: sam; +Cc: viro, jbeulich, mingo, rostedt, srostedt, linux-kernel, sparclinux From: Sam Ravnborg <sam@ravnborg.org> Date: Tue, 6 Jan 2009 14:34:42 +0100 > But I never managed to really understand what the btfixup thing is all > about and has then been sidetracked by funnier stuff. It's recording relocations that get fixed up at boot time. The way it works is that each btfixup emits a reference to a symbol that will be unresolved. The btfixup tool under arch/sparc/boot/ scans the unlinked kernel image, generates dummy symbol definitions into a foo.s file so that the kernel can be linked, and builds the btfixup tables so the kernel can patch up these relocations at boot time. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: ftrace breaks sparc64 build 2009-01-06 11:35 ` Al Viro 2009-01-06 12:39 ` Jan Beulich 2009-01-06 13:34 ` Sam Ravnborg @ 2009-01-08 9:28 ` Jan Beulich 2 siblings, 0 replies; 37+ messages in thread From: Jan Beulich @ 2009-01-08 9:28 UTC (permalink / raw) To: Al Viro Cc: David S. Miller, Ingo Molnar, rostedt, Sam Ravnborg, Steven Rostedt, LKML, sparclinux >>> Al Viro <viro@ZenIV.linux.org.uk> 06.01.09 12:35 >>> >On Tue, Jan 06, 2009 at 07:53:04AM +0000, Jan Beulich wrote: >> The others look like a tools side behavioral difference, as I never saw any >> such. Is this problem sparc32-specific (I tested x86 and ia64 only)? What's >> the binutils version used? > >2.18.50.0.6. > >And no, it's not tools side. What it is, AFAICT, is that sparc32 has >LDFLAGS_vmlinux = -r, which leaves a metric arseload of relocs that >wouldn't have survived into vmlinux otherwise. Look at .rela__ksymtab >in .tmp_vmlinux1, for example... So that should help: --- a/init/Kconfig +++ b/init/Kconfig @@ -588,6 +588,8 @@ config KALLSYMS_STRIP_GENERATED bool "Strip machine generated symbols from kallsyms" depends on KALLSYMS_ALL + # This doesn't work with -r in LDFLAGS_vmlinux. + depends on !SPARC || SPARC64 default y help Say N if you want kallsyms to retain even machine generated symbols. ^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2009-01-08 9:28 UTC | newest] Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-01-05 18:19 ftrace breaks sparc64 build Sam Ravnborg 2009-01-05 19:31 ` Steven Rostedt 2009-01-05 19:42 ` [PATCH] sparc: make proces_ver_nack a bit more readable Steven Rostedt 2009-01-05 19:46 ` Steven Rostedt 2009-01-05 19:56 ` Steven Rostedt 2009-01-05 20:07 ` Sam Ravnborg 2009-01-05 20:08 ` Sam Ravnborg 2009-01-06 18:23 ` David Miller 2009-01-05 19:54 ` ftrace breaks sparc64 build Sam Ravnborg 2009-01-05 20:05 ` Steven Rostedt 2009-01-05 21:31 ` Sam Ravnborg 2009-01-05 21:52 ` Steven Rostedt 2009-01-05 22:01 ` Sam Ravnborg 2009-01-05 22:14 ` Steven Rostedt 2009-01-05 23:11 ` Heiko Carstens 2009-01-06 2:07 ` Steven Rostedt 2009-01-06 9:36 ` Heiko Carstens 2009-01-06 4:30 ` Steven Rostedt 2009-01-06 9:45 ` Heiko Carstens 2009-01-06 18:32 ` David Miller 2009-01-06 18:52 ` Steven Rostedt 2009-01-06 19:01 ` David Miller 2009-01-06 19:52 ` Sam Ravnborg 2009-01-06 20:02 ` David Miller 2009-01-05 20:30 ` [PATCH] module: clean up initialization of variable Steven Rostedt 2009-01-05 22:59 ` Harvey Harrison 2009-01-06 1:22 ` Rusty Russell 2009-01-06 2:02 ` Steven Rostedt 2009-01-05 19:48 ` ftrace breaks sparc64 build Al Viro 2009-01-05 19:55 ` Sam Ravnborg 2009-01-06 7:53 ` Jan Beulich 2009-01-06 11:35 ` Al Viro 2009-01-06 12:39 ` Jan Beulich 2009-01-06 13:34 ` Sam Ravnborg 2009-01-06 15:52 ` Al Viro 2009-01-06 18:39 ` David Miller 2009-01-08 9:28 ` Jan Beulich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).