[PATCH v2 RESEND] tracing: probeevent: Correctly update remaining space in dynamic area
Commit 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area") improved the string fetching mechanism by returning the number of required bytes after copying the argument to the dynamic area. However, this return value is now only used to increment the pointer inside the dynamic area but misses updating the 'maxlen' variable which indicates the remaining space in the dynamic area. This means that fetch_store_string() always reads the *total* size of the dynamic area from the data_loc pointer instead of the *remaining* size (and passes it along to strncpy_from_{user,unsafe}) even if we're already about to copy data into the middle of the dynamic area. Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area") Signed-off-by: Andreas Ziegler Acked-by: Masami Hiramatsu --- v2: follow coding guidelines for braces kernel/trace/trace_probe_tmpl.h | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h index 5c56afc17cf8..4737bb8c07a3 100644 --- a/kernel/trace/trace_probe_tmpl.h +++ b/kernel/trace/trace_probe_tmpl.h @@ -180,10 +180,12 @@ store_trace_args(void *data, struct trace_probe *tp, struct pt_regs *regs, if (unlikely(arg->dynamic)) *dl = make_data_loc(maxlen, dyndata - base); ret = process_fetch_insn(arg->code, regs, dl, base); - if (unlikely(ret < 0 && arg->dynamic)) + if (unlikely(ret < 0 && arg->dynamic)) { *dl = make_data_loc(0, dyndata - base); - else + } else { dyndata += ret; + maxlen -= ret; + } } } -- 2.17.1
[PATCH v2] tracing: probeevent: Correctly update remaining space in dynamic area
Commit 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area") improved the string fetching mechanism by returning the number of required bytes after copying the argument to the dynamic area. However, this return value is now only used to increment the pointer inside the dynamic area but misses updating the 'maxlen' variable which indicates the remaining space in the dynamic area. This means that fetch_store_string() always reads the *total* size of the dynamic area from the data_loc pointer instead of the *remaining* size (and passes it along to strncpy_from_{user,unsafe}) even if we're already about to copy data into the middle of the dynamic area. Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area") Signed-off-by: Andreas Ziegler Acked-by: Masami Hiramatsu --- v2: follow coding guidelines for braces kernel/trace/trace_probe_tmpl.h | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h index 5c56afc17cf8..4737bb8c07a3 100644 --- a/kernel/trace/trace_probe_tmpl.h +++ b/kernel/trace/trace_probe_tmpl.h @@ -180,10 +180,12 @@ store_trace_args(void *data, struct trace_probe *tp, struct pt_regs *regs, if (unlikely(arg->dynamic)) *dl = make_data_loc(maxlen, dyndata - base); ret = process_fetch_insn(arg->code, regs, dl, base); - if (unlikely(ret < 0 && arg->dynamic)) + if (unlikely(ret < 0 && arg->dynamic)) { *dl = make_data_loc(0, dyndata - base); - else + } else { dyndata += ret; + maxlen -= ret; + } } } -- 2.17.1
[PATCH] tracing: probeevent: Correctly update remaining space in dynamic area
Commit 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area") improved the string fetching mechanism by returning the number of required bytes after copying the argument to the dynamic area. However, this return value is now only used to increment the pointer inside the dynamic area but misses updating the 'maxlen' variable which indicates the remaining space in the dynamic area. This means that fetch_store_string() always reads the *total* size of the dynamic area from the data_loc pointer instead of the *remaining* size (and passes it along to strncpy_from_{user,unsafe}) even if we're already about to copy data into the middle of the dynamic area. Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area") Signed-off-by: Andreas Ziegler --- kernel/trace/trace_probe_tmpl.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h index 5c56afc17cf8..0cf953e47584 100644 --- a/kernel/trace/trace_probe_tmpl.h +++ b/kernel/trace/trace_probe_tmpl.h @@ -182,8 +182,10 @@ store_trace_args(void *data, struct trace_probe *tp, struct pt_regs *regs, ret = process_fetch_insn(arg->code, regs, dl, base); if (unlikely(ret < 0 && arg->dynamic)) *dl = make_data_loc(0, dyndata - base); - else + else { dyndata += ret; + maxlen -= ret; + } } } -- 2.17.1
Re: [PATCH v2] tracing/uprobes: Fix output for multiple string arguments
On 17. Jan 2019, at 15:51, Steven Rostedt wrote: > > On Thu, 17 Jan 2019 15:29:09 +0100 > Andreas Ziegler wrote: > >> From my side it's basically good to go, but I just realized that there >> is no "Fixes: " tag in it but the problem it fixes goes back to >> 5baaa59ef09e ("tracing/probes: Implement 'memory' fetch method for >> uprobes") from 2013 (and is present in all current longterm and stable >> kernels)... should I add a tag and resend a v3? >> > > I was just about to ask you to provide the fixes tag. No need to > resend, I'll just add it manually. I asked Masami to resend because it > was a patch series. If it was just a single patch, I would have just > asked what the fixes was for that particular patch. I can handle one > patch, but not more than one ;-) > > -- Steve One more thing maybe, the code in question was rewritten by Masami in 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area”) which is in v4.20 but the problem traces back to the original commit mentioned above, so it will need manual so we will need a slightly modified version for earlier kernels. Regards, Andreas
Re: [PATCH v2] tracing/uprobes: Fix output for multiple string arguments
On 1/17/19 3:20 PM, Steven Rostedt wrote: On Thu, 17 Jan 2019 16:58:07 +0900 Masami Hiramatsu wrote: Ah, I got it. Hmm, in that case, I have to update my patch in the previous mail. Anyway, Acked-by: Masami Hiramatsu So this patch is good to go? If so, I'll pull it in and start testing it. I'm currently traveling (as you probably received my OoO emails), but I can still do a bit of work remotely. -- Steve From my side it's basically good to go, but I just realized that there is no "Fixes: " tag in it but the problem it fixes goes back to 5baaa59ef09e ("tracing/probes: Implement 'memory' fetch method for uprobes") from 2013 (and is present in all current longterm and stable kernels)... should I add a tag and resend a v3? Thanks, Andreas
Re: uprobes: bug in comm/string output?
Hi, On 1/17/19 10:47 AM, Masami Hiramatsu wrote: On Thu, 17 Jan 2019 09:08:41 +0100 Andreas Ziegler wrote: On 17.01.19 09:00, Masami Hiramatsu wrote: On Thu, 17 Jan 2019 15:13:09 +0900 Masami Hiramatsu wrote: On Wed, 16 Jan 2019 11:16:07 +0100 Andreas Ziegler wrote: I went into this a bit deeper today, and right now it is simply failing to parse the code because there is no FETCH_OP_COMM case in process_fetch_insn() for uprobes so that will return -EILSEQ, leading to a make_data_loc(0, ...) in store_trace_args(). If we just add FETCH_OP_COMM and let val point to current->comm (that's what trace_kprobe.c does), we get an -EFAULT return value from fetch_store_string because strncpy_from_user() checks if the argument is in user space. Correct. I missed to add OP_COMM support. And uprobe's fetch_store_string is only for user space strings. So I think we might need a special case for that, something like FETCH_OP_ST_COMM_STRING which is only used for FETCH_OP_COMM and copies current->comm over to the dynamic area. The implementation could be similar to the old fetch_comm_string implementation before your rewrite. Hmm, instead, I would like to add current->comm checker and only allows to copy that. That would be simpler and enough. Could you test below patch? tracing: uprobes: Re-enable $comm support for uprobe events From: Masami Hiramatsu Since commit 533059281ee5 ("tracing: probeevent: Introduce new argument fetching code") dropped the $comm support from uprobe events, this re-enable it. this should read 're-enables'. For $comm support, use strncpy() instead of strncpy_from_user() ^ we're using strlcpy(), not strncpy(). to copy current task's comm. Because it is in the kernel space, strncpy_from_user() always fails to copy the comm. This also use strlen() instead of strlen_user() to measure the ^^ 'uses', and the function should be 'strnlen_user()'. length of the comm. Signed-off-by: Masami Hiramatsu Reported-by: Andreas Ziegler --- kernel/trace/trace_uprobe.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index e335576b9411..97d134e83e0f 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -156,7 +156,10 @@ fetch_store_string(unsigned long addr, void *dest, void *base) if (unlikely(!maxlen)) return -ENOMEM; - ret = strncpy_from_user(dst, src, maxlen); + if (addr == (unsigned long)current->comm) + ret = strlcpy(dst, current->comm, maxlen); + else + ret = strncpy_from_user(dst, src, maxlen); if (ret >= 0) { if (ret == maxlen) dst[ret - 1] = '\0'; @@ -173,7 +176,10 @@ fetch_store_strlen(unsigned long addr) int len; void __user *vaddr = (void __force __user *) addr; - len = strnlen_user(vaddr, MAX_STRING_SIZE); + if (addr == (unsigned long)current->comm) + len = strlen(current->comm); To balance with the strnlen_user, we must increse the len in this block. (strlen doesn't count the final '\0', but strnlen_user counts it) yes, we need to add a '+ 1' here. With the typos and this one fixed, this is Acked-by: Andreas Ziegler Thank you for fixing typo and Ack :) Thanks you, > Thank you, + else + len = strnlen_user(vaddr, MAX_STRING_SIZE); return (len > MAX_STRING_SIZE) ? 0 : len; } @@ -213,6 +219,9 @@ process_fetch_insn(struct fetch_insn *code, struct pt_regs *regs, void *dest, case FETCH_OP_IMM: val = code->immediate; break; + case FETCH_OP_COMM: + val = (unsigned long)current->comm; + break; case FETCH_OP_FOFFS: val = translate_user_vaddr(code->immediate); break; as the original commit breaking $comm support was merged for v4.20 (which is a stable kernel) and the wrong behaviour with multiple strings exists in all longterm/stable releases (tested back to v4.4), do you think this should be going into a stable release once it's merged? I added Greg as he might know the answer to that. Thanks, Andreas smime.p7s Description: S/MIME Cryptographic Signature
[PATCH] tracing: uprobes: fix typo in pr_fmt string
The subsystem-specific message prefix for uprobes was also "trace_kprobe: " instead of "trace_uprobe: " as described in the original commit message. Fixes: 7257634135c24 ("tracing/probe: Show subsystem name in messages") Signed-off-by: Andreas Ziegler --- kernel/trace/trace_uprobe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index e335576b9411..19a1a8e19062 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -5,7 +5,7 @@ * Copyright (C) IBM Corporation, 2010-2012 * Author: Srikar Dronamraju */ -#define pr_fmt(fmt)"trace_kprobe: " fmt +#define pr_fmt(fmt)"trace_uprobe: " fmt #include #include -- 2.17.1
Re: uprobes: bug in comm/string output?
On 17.01.19 09:00, Masami Hiramatsu wrote: > On Thu, 17 Jan 2019 15:13:09 +0900 > Masami Hiramatsu wrote: > >> On Wed, 16 Jan 2019 11:16:07 +0100 >> Andreas Ziegler wrote: >> >>> >>> I went into this a bit deeper today, and right now it is simply failing >>> to parse the code because there is no FETCH_OP_COMM case in >>> process_fetch_insn() for uprobes so that will return -EILSEQ, leading to >>> a make_data_loc(0, ...) in store_trace_args(). If we just add >>> FETCH_OP_COMM and let val point to current->comm (that's what >>> trace_kprobe.c does), we get an -EFAULT return value from >>> fetch_store_string because strncpy_from_user() checks if the argument is >>> in user space. >> >> Correct. I missed to add OP_COMM support. And uprobe's fetch_store_string >> is only for user space strings. >> >>> So I think we might need a special case for that, something like >>> FETCH_OP_ST_COMM_STRING which is only used for FETCH_OP_COMM and copies >>> current->comm over to the dynamic area. The implementation could be >>> similar to the old fetch_comm_string implementation before your rewrite. >> >> Hmm, instead, I would like to add current->comm checker and only allows >> to copy that. That would be simpler and enough. >> >> Could you test below patch? >> >> >> tracing: uprobes: Re-enable $comm support for uprobe events >> >> From: Masami Hiramatsu >> >> Since commit 533059281ee5 ("tracing: probeevent: Introduce new >> argument fetching code") dropped the $comm support from uprobe >> events, this re-enable it. this should read 're-enables'. >> >> For $comm support, use strncpy() instead of strncpy_from_user() ^ we're using strlcpy(), not strncpy(). >> to copy current task's comm. Because it is in the kernel space, >> strncpy_from_user() always fails to copy the comm. >> This also use strlen() instead of strlen_user() to measure the ^^ 'uses', and the function should be 'strnlen_user()'. >> length of the comm. >> >> Signed-off-by: Masami Hiramatsu >> Reported-by: Andreas Ziegler >> --- >> kernel/trace/trace_uprobe.c | 13 +++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c >> index e335576b9411..97d134e83e0f 100644 >> --- a/kernel/trace/trace_uprobe.c >> +++ b/kernel/trace/trace_uprobe.c >> @@ -156,7 +156,10 @@ fetch_store_string(unsigned long addr, void *dest, void >> *base) >> if (unlikely(!maxlen)) >> return -ENOMEM; >> >> -ret = strncpy_from_user(dst, src, maxlen); >> +if (addr == (unsigned long)current->comm) >> +ret = strlcpy(dst, current->comm, maxlen); >> +else >> +ret = strncpy_from_user(dst, src, maxlen); >> if (ret >= 0) { >> if (ret == maxlen) >> dst[ret - 1] = '\0'; >> @@ -173,7 +176,10 @@ fetch_store_strlen(unsigned long addr) >> int len; >> void __user *vaddr = (void __force __user *) addr; >> >> -len = strnlen_user(vaddr, MAX_STRING_SIZE); >> +if (addr == (unsigned long)current->comm) >> +len = strlen(current->comm); > > To balance with the strnlen_user, we must increse the len in this block. > (strlen doesn't count the final '\0', but strnlen_user counts it) > yes, we need to add a '+ 1' here. With the typos and this one fixed, this is Acked-by: Andreas Ziegler > Thank you, > >> +else >> +len = strnlen_user(vaddr, MAX_STRING_SIZE); >> >> return (len > MAX_STRING_SIZE) ? 0 : len; >> } >> @@ -213,6 +219,9 @@ process_fetch_insn(struct fetch_insn *code, struct >> pt_regs *regs, void *dest, >> case FETCH_OP_IMM: >> val = code->immediate; >> break; >> +case FETCH_OP_COMM: >> +val = (unsigned long)current->comm; >> +break; >> case FETCH_OP_FOFFS: >> val = translate_user_vaddr(code->immediate); >> break; > >
Re: [PATCH v2] tracing/uprobes: Fix output for multiple string arguments
On 17.01.19 07:01, Masami Hiramatsu wrote: > On Wed, 16 Jan 2019 15:16:29 +0100 > Andreas Ziegler wrote: > >> When printing multiple uprobe arguments as strings the output for the >> earlier arguments would also include all later string arguments. >> >> This is best explained in an example: >> >> Consider adding a uprobe to a function receiving two strings as >> parameters which is at offset 0xa0 in strlib.so and we want to print >> both parameters when the uprobe is hit (on x86_64): >> >> $ echo 'p:func /lib/strlib.so:0xa0 +0(%di):string +0(%si):string' > \ >> /sys/kernel/debug/tracing/uprobe_events >> >> When the function is called as func("foo", "bar") and we hit the probe, >> the trace file shows a line like the following: >> >> [...] func: (0x7f7e683706a0) arg1="foobar" arg2="bar" >> >> Note the extra "bar" printed as part of arg1. This behaviour stacks up >> for additional string arguments. >> >> The strings are stored in a dynamically growing part of the uprobe >> buffer by fetch_store_string() after copying them from userspace via >> strncpy_from_user(). The return value of strncpy_from_user() is then >> directly used as the required size for the string. However, this does >> not take the terminating null byte into account as the documentation >> for strncpy_from_user() cleary states that it "[...] returns the >> length of the string (not including the trailing NUL)" even though the >> null byte will be copied to the destination. >> >> Therefore, subsequent calls to fetch_store_string() will overwrite >> the terminating null byte of the most recently fetched string with >> the first character of the current string, leading to the >> "accumulation" of strings in earlier arguments in the output. >> >> Fix this by incrementing the return value of strncpy_from_user() by >> one if we did not hit the maximum buffer size. >> > > Yeah, I had eventually same conclusion. However, you also have to increse > the return value of fetch_store_strlen() too. (And I found another issue) > I don't think we need to increase that since the documentation for strnlen_user() says that it "[r]eturns the size of the string INCLUDING the terminating NUL." so its return value will already be one more than that of strncpy_from_user(). Thanks, Andreas > Could you fix fetch_store_strlen in the same patch? > > Thank you, > >> Signed-off-by: Andreas Ziegler >> --- >> v2: removed a wrong check for (ret > 0) >> >> kernel/trace/trace_uprobe.c | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c >> index e335576b9411..3a1d5ab6b4ba 100644 >> --- a/kernel/trace/trace_uprobe.c >> +++ b/kernel/trace/trace_uprobe.c >> @@ -160,6 +160,13 @@ fetch_store_string(unsigned long addr, void *dest, void >> *base) >> if (ret >= 0) { >> if (ret == maxlen) >> dst[ret - 1] = '\0'; >> +else >> +/* >> + * Include the terminating null byte. In this case it >> + * was copied by strncpy_from_user but not accounted >> + * for in ret. >> + */ >> +ret++; >> *(u32 *)dest = make_data_loc(ret, (void *)dst - base); >> } >> >> -- >> 2.17.1 >> > >
[PATCH v2] tracing/uprobes: Fix output for multiple string arguments
When printing multiple uprobe arguments as strings the output for the earlier arguments would also include all later string arguments. This is best explained in an example: Consider adding a uprobe to a function receiving two strings as parameters which is at offset 0xa0 in strlib.so and we want to print both parameters when the uprobe is hit (on x86_64): $ echo 'p:func /lib/strlib.so:0xa0 +0(%di):string +0(%si):string' > \ /sys/kernel/debug/tracing/uprobe_events When the function is called as func("foo", "bar") and we hit the probe, the trace file shows a line like the following: [...] func: (0x7f7e683706a0) arg1="foobar" arg2="bar" Note the extra "bar" printed as part of arg1. This behaviour stacks up for additional string arguments. The strings are stored in a dynamically growing part of the uprobe buffer by fetch_store_string() after copying them from userspace via strncpy_from_user(). The return value of strncpy_from_user() is then directly used as the required size for the string. However, this does not take the terminating null byte into account as the documentation for strncpy_from_user() cleary states that it "[...] returns the length of the string (not including the trailing NUL)" even though the null byte will be copied to the destination. Therefore, subsequent calls to fetch_store_string() will overwrite the terminating null byte of the most recently fetched string with the first character of the current string, leading to the "accumulation" of strings in earlier arguments in the output. Fix this by incrementing the return value of strncpy_from_user() by one if we did not hit the maximum buffer size. Signed-off-by: Andreas Ziegler --- v2: removed a wrong check for (ret > 0) kernel/trace/trace_uprobe.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index e335576b9411..3a1d5ab6b4ba 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -160,6 +160,13 @@ fetch_store_string(unsigned long addr, void *dest, void *base) if (ret >= 0) { if (ret == maxlen) dst[ret - 1] = '\0'; + else + /* +* Include the terminating null byte. In this case it +* was copied by strncpy_from_user but not accounted +* for in ret. +*/ + ret++; *(u32 *)dest = make_data_loc(ret, (void *)dst - base); } -- 2.17.1
Re: [PATCH] tracing/uprobes: Fix output for multiple string arguments
On 1/16/19 2:40 PM, Steven Rostedt wrote: On Wed, 16 Jan 2019 10:41:12 +0100 Andreas Ziegler wrote: When printing multiple uprobe arguments as strings the output for the earlier arguments would also include all later string arguments. This is best explained in an example: Consider adding a uprobe to a function receiving two strings as parameters which is at offset 0xa0 in strlib.so and we want to print both parameters when the uprobe is hit (on x86_64): $ echo 'p:func /lib/strlib.so:0xa0 +0(%di):string +0(%si):string' > \ /sys/kernel/debug/tracing/uprobe_events When the function is called as func("foo", "bar") and we hit the probe, the trace file shows a line like the following: [...] func: (0x7f7e683706a0) arg1="foobar" arg2="bar" Note the extra "bar" printed as part of arg1. This behaviour stacks up for additional string arguments. The strings are stored in a dynamically growing part of the uprobe buffer by fetch_store_string() after copying them from userspace via strncpy_from_user(). The return value of strncpy_from_user() is then directly used as the required size for the string. However, this does not take the terminating null byte into account as the documentation for strncpy_from_user() cleary states that it "[...] returns the length of the string (not including the trailing NUL)" even though the null byte will be copied to the destination. Therefore, subsequent calls to fetch_store_string() will overwrite the terminating null byte of the most recently fetched string with the first character of the current string, leading to the "accumulation" of strings in earlier arguments in the output. Fix this by incrementing the return value of strncpy_from_user() by one if we did not hit the maximum buffer size. Signed-off-by: Andreas Ziegler --- kernel/trace/trace_uprobe.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index e335576b9411..dfb9bbc7fd82 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -160,6 +160,13 @@ fetch_store_string(unsigned long addr, void *dest, void *base) if (ret >= 0) { if (ret == maxlen) dst[ret - 1] = '\0'; + else if (ret > 0) Do we need the ret > 0 check? What if the value is ""? Doesn't that cause the same issue? -- Steve yes, it does. With this patch an empty string will also print "(fault)", I missed that, sorry. I'll send a v2. Thanks, Andreas + /* +* Include the terminating null byte. In this case it +* was copied by strncpy_from_user but not accounted +* for in ret. +*/ + ret++; *(u32 *)dest = make_data_loc(ret, (void *)dst - base); } smime.p7s Description: S/MIME Cryptographic Signature
Re: uprobes: bug in comm/string output?
Hi, thanks for your reply! On 1/16/19 11:00 AM, Masami Hiramatsu wrote: On Tue, 15 Jan 2019 14:36:48 +0100 Andreas Ziegler wrote: Hi again, On 1/14/19 1:38 PM, Andreas Ziegler wrote: Hi, I've been playing around with uprobes today and found the following weird behaviour/output when using more than one string argument (or using the $comm argument). This was run on a v4.20 mainline build on Ubuntu 18.04. root@ubuntu1810:~# uname -a Linux ubuntu1810 4.20.0-042000-generic #201812232030 SMP Mon Dec 24 01:32:58 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux I'm trying to track calls to dlopen so I'm looking up the correct offset in libdl.so: root@ubuntu1810:/sys/kernel/debug/tracing# readelf -s /lib/x86_64-linux-gnu/libdl-2.28.so | grep dlopen 34: 12a0 133 FUNCGLOBAL DEFAULT 14 dlopen@@GLIBC_2.2.5 Then I'm creating a uprobe with two prints of $comm and two prints of the first argument to dlopen, and enable that probe. The '/root/test' program only does a dlopen("libc.so.6", RTLD_LAZY) in main(). root@ubuntu1810:/sys/kernel/debug/tracing# echo 'p:dlopen /lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm +0(%di):string +0(%di):string' > uprobe_events root@ubuntu1810:/sys/kernel/debug/tracing# echo 1 > events/uprobes/dlopen/enable root@ubuntu1810:/sys/kernel/debug/tracing# /root/test The trace output looks like this: root@ubuntu1810:/sys/kernel/debug/tracing# cat trace # tracer: nop # # _-=> irqs-off # / _=> need-resched #| / _---=> hardirq/softirq #|| / _--=> preempt-depth #||| / delay # TASK-PID CPU# TIMESTAMP FUNCTION # | | | | | test-1617 [000] d... 1237.959168: dlopen: (0x7fbd5272e2a0) arg1=(fault) arg2=(fault) arg3="libc.so.6libc.so.6" arg4="libc.so.6" That's very weird for two reasons: - fetching $comm seems to fail with an invalid pointer - arg3 contains the text twice (if I add another print of the argument, arg3 will contain the wanted string three times, arg4 two times and the last argument will contain it once). at least for the second problem I think I found the answer, and for the first problem I have a suspicion (see last paragraph for that). OK, this looks broken. Thank you very much for reporting it! BTW, I tried to reproduce it with kprobe event, but it seems working well. e.g. # echo 'p ksys_chdir $comm $comm +0(%di):string +0(%di):string' > kprobe_events # echo 1 > events/kprobes/enable # cd /sys/kernel/debug/tracing # cat trace sh-812 [003] ...1 229.344360: p_ksys_chdir_0: (ksys_chdir+0x0/0xc0) arg1="sh" arg2="sh" arg3="/sys/kernel/debug/tracing" arg4="/sys/kernel/debug/tracing" So, it might be an issue on uprobe_event. yes, kprobes work because they use strncpy_from_unsafe which *includes* the null byte in its return value... the fact that both are called strncpy_* but behave differently is really annoying... I just sent a patch for this case half an hour ago which simply adds 1 to the returned value for uprobes if it didn't hit the maximum allowed length. For this, I installed a uprobe for libdl.so/dlopen once again, the command would be 'p:dlopen /lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm' so it should print the process name twice (using a kernel v4.18 on Ubuntu 18.10). The code which prints the collected data (print_type_string, defined by PRINT_TYPE_FUNC_NAME(string) in kernel/trace/trace_probe.c) is the following, it simply passes the respective data to trace_seq_printf with a "%s" format string: int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s, void *data, void *ent) { int len = *(u32 *)data >> 16; if (!len) trace_seq_puts(s, "(fault)"); else trace_seq_printf(s, "\"%s\"", (const char *)get_loc_data(data, ent)); return !trace_seq_has_overflowed(s); } I dug into that function with KGDB and found the following: 'data' holds the size and offset for the member in question, which is 0xf and 0x18, respectively. 'ent' holds the base address for event. When we print the string at ent + 0x18, we can see that the output for 'arg1' will be "update-notifierupdate-notifier" Thread 511 hit Breakpoint 6, print_type_string (s=0x880078fd1090, name=0x880078fe4458 "arg1", data=0x88007d01f05c, ent=0x88007d01f04c) at /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c:67 67 in /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c gdb$ p *(uint32_t *) data $46 = 0xf0018 gdb$ p ent $47 = (void *) 0x88007d01f04c gdb$ p ((char *)ent + 0x18) $48 = 0x88
[PATCH] tracing/uprobes: Fix output for multiple string arguments
When printing multiple uprobe arguments as strings the output for the earlier arguments would also include all later string arguments. This is best explained in an example: Consider adding a uprobe to a function receiving two strings as parameters which is at offset 0xa0 in strlib.so and we want to print both parameters when the uprobe is hit (on x86_64): $ echo 'p:func /lib/strlib.so:0xa0 +0(%di):string +0(%si):string' > \ /sys/kernel/debug/tracing/uprobe_events When the function is called as func("foo", "bar") and we hit the probe, the trace file shows a line like the following: [...] func: (0x7f7e683706a0) arg1="foobar" arg2="bar" Note the extra "bar" printed as part of arg1. This behaviour stacks up for additional string arguments. The strings are stored in a dynamically growing part of the uprobe buffer by fetch_store_string() after copying them from userspace via strncpy_from_user(). The return value of strncpy_from_user() is then directly used as the required size for the string. However, this does not take the terminating null byte into account as the documentation for strncpy_from_user() cleary states that it "[...] returns the length of the string (not including the trailing NUL)" even though the null byte will be copied to the destination. Therefore, subsequent calls to fetch_store_string() will overwrite the terminating null byte of the most recently fetched string with the first character of the current string, leading to the "accumulation" of strings in earlier arguments in the output. Fix this by incrementing the return value of strncpy_from_user() by one if we did not hit the maximum buffer size. Signed-off-by: Andreas Ziegler --- kernel/trace/trace_uprobe.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index e335576b9411..dfb9bbc7fd82 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -160,6 +160,13 @@ fetch_store_string(unsigned long addr, void *dest, void *base) if (ret >= 0) { if (ret == maxlen) dst[ret - 1] = '\0'; + else if (ret > 0) + /* +* Include the terminating null byte. In this case it +* was copied by strncpy_from_user but not accounted +* for in ret. +*/ + ret++; *(u32 *)dest = make_data_loc(ret, (void *)dst - base); } -- 2.17.1
Re: uprobes: bug in comm/string output?
Hi again, On 1/14/19 1:38 PM, Andreas Ziegler wrote: Hi, I've been playing around with uprobes today and found the following weird behaviour/output when using more than one string argument (or using the $comm argument). This was run on a v4.20 mainline build on Ubuntu 18.04. root@ubuntu1810:~# uname -a Linux ubuntu1810 4.20.0-042000-generic #201812232030 SMP Mon Dec 24 01:32:58 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux I'm trying to track calls to dlopen so I'm looking up the correct offset in libdl.so: root@ubuntu1810:/sys/kernel/debug/tracing# readelf -s /lib/x86_64-linux-gnu/libdl-2.28.so | grep dlopen 34: 12a0 133 FUNCGLOBAL DEFAULT 14 dlopen@@GLIBC_2.2.5 Then I'm creating a uprobe with two prints of $comm and two prints of the first argument to dlopen, and enable that probe. The '/root/test' program only does a dlopen("libc.so.6", RTLD_LAZY) in main(). root@ubuntu1810:/sys/kernel/debug/tracing# echo 'p:dlopen /lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm +0(%di):string +0(%di):string' > uprobe_events root@ubuntu1810:/sys/kernel/debug/tracing# echo 1 > events/uprobes/dlopen/enable root@ubuntu1810:/sys/kernel/debug/tracing# /root/test The trace output looks like this: root@ubuntu1810:/sys/kernel/debug/tracing# cat trace # tracer: nop # # _-=> irqs-off # / _=> need-resched #| / _---=> hardirq/softirq #|| / _--=> preempt-depth #||| / delay # TASK-PID CPU# TIMESTAMP FUNCTION # | | | | | test-1617 [000] d... 1237.959168: dlopen: (0x7fbd5272e2a0) arg1=(fault) arg2=(fault) arg3="libc.so.6libc.so.6" arg4="libc.so.6" That's very weird for two reasons: - fetching $comm seems to fail with an invalid pointer - arg3 contains the text twice (if I add another print of the argument, arg3 will contain the wanted string three times, arg4 two times and the last argument will contain it once). at least for the second problem I think I found the answer, and for the first problem I have a suspicion (see last paragraph for that). For this, I installed a uprobe for libdl.so/dlopen once again, the command would be 'p:dlopen /lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm' so it should print the process name twice (using a kernel v4.18 on Ubuntu 18.10). The code which prints the collected data (print_type_string, defined by PRINT_TYPE_FUNC_NAME(string) in kernel/trace/trace_probe.c) is the following, it simply passes the respective data to trace_seq_printf with a "%s" format string: int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s, void *data, void *ent) { int len = *(u32 *)data >> 16; if (!len) trace_seq_puts(s, "(fault)"); else trace_seq_printf(s, "\"%s\"", (const char *)get_loc_data(data, ent)); return !trace_seq_has_overflowed(s); } I dug into that function with KGDB and found the following: 'data' holds the size and offset for the member in question, which is 0xf and 0x18, respectively. 'ent' holds the base address for event. When we print the string at ent + 0x18, we can see that the output for 'arg1' will be "update-notifierupdate-notifier" Thread 511 hit Breakpoint 6, print_type_string (s=0x880078fd1090, name=0x880078fe4458 "arg1", data=0x88007d01f05c, ent=0x88007d01f04c) at /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c:67 67 in /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c gdb$ p *(uint32_t *) data $46 = 0xf0018 gdb$ p ent $47 = (void *) 0x88007d01f04c gdb$ p ((char *)ent + 0x18) $48 = 0x88007d01f064 "update-notifierupdate-notifier" Moving on printing 'arg2' (e.g., the other $comm string). Here we see that the string in question is at offset 0x27, and if we print that, we see the correct "update-notifier". Thread 511 hit Breakpoint 6, print_type_string (s=0x880078fd1090, name=0x880078fe4d70 "arg2", data=0x88007d01f060, ent=0x88007d01f04c) at /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c:67 67 in /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c gdb$ p *(uint32_t *) data $49 = 0xf0027 gdb$ p ((char *)ent + 0x27) $50 = 0x88007d01f073 "update-notifier" Looking at the bytes in memory and the offsets it becomes clear that there is no \0 byte at the end of the first entry (which would need to be at address 0x88007d01f064 + 0xf = 0x88007d01f073 but instead that's the start address of the second entry which simply gets consumed by the (first) "%s" as well. gdb$ x/32x ent 0x88007d01f04c: 0x00010592 0x2143
uprobes: bug in comm/string output?
Hi, I've been playing around with uprobes today and found the following weird behaviour/output when using more than one string argument (or using the $comm argument). This was run on a v4.20 mainline build on Ubuntu 18.04. root@ubuntu1810:~# uname -a Linux ubuntu1810 4.20.0-042000-generic #201812232030 SMP Mon Dec 24 01:32:58 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux I'm trying to track calls to dlopen so I'm looking up the correct offset in libdl.so: root@ubuntu1810:/sys/kernel/debug/tracing# readelf -s /lib/x86_64-linux-gnu/libdl-2.28.so | grep dlopen 34: 12a0 133 FUNCGLOBAL DEFAULT 14 dlopen@@GLIBC_2.2.5 Then I'm creating a uprobe with two prints of $comm and two prints of the first argument to dlopen, and enable that probe. The '/root/test' program only does a dlopen("libc.so.6", RTLD_LAZY) in main(). root@ubuntu1810:/sys/kernel/debug/tracing# echo 'p:dlopen /lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm +0(%di):string +0(%di):string' > uprobe_events root@ubuntu1810:/sys/kernel/debug/tracing# echo 1 > events/uprobes/dlopen/enable root@ubuntu1810:/sys/kernel/debug/tracing# /root/test The trace output looks like this: root@ubuntu1810:/sys/kernel/debug/tracing# cat trace # tracer: nop # # _-=> irqs-off # / _=> need-resched #| / _---=> hardirq/softirq #|| / _--=> preempt-depth #||| / delay # TASK-PID CPU# TIMESTAMP FUNCTION # | | | | | test-1617 [000] d... 1237.959168: dlopen: (0x7fbd5272e2a0) arg1=(fault) arg2=(fault) arg3="libc.so.6libc.so.6" arg4="libc.so.6" That's very weird for two reasons: - fetching $comm seems to fail with an invalid pointer - arg3 contains the text twice (if I add another print of the argument, arg3 will contain the wanted string three times, arg4 two times and the last argument will contain it once). On the "standard" kernel shipped with Ubuntu 18.04 (4.18) printing $comm works but also "accumulates" (in lack of a better word) the later string arguments in the earlier output arguments, so for the probe above arg1 would be "testtestlibc.so.6libc.so.6", arg2 would be "testlibc.so.6libc.so.6" and arg3/arg4 will be the same as in the output above. Is there anything in the documentation why multiple string outputs might not work with uprobes? Am I installing the uprobe wrong? Or is this a bug? I found that the kprobe selftest at tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc contains a similar situation for kprobes (it prints the parameter two times) which works fine on the same system. Regards, Andreas smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] regmap: make LZO cache optional
Hi Jonas, I noticed your patch 'regmap: make LZO cache optional' as it recently showed up in linux-next. In your patch, you modify drivers/base/regmap/regcache.c by adding an #if IS_ENABLED() statement. However, this statement contains a spelling error, as it references REGCHACHE_COMPRESSED instead of REGCACHE_COMPRESSED (note the extra H). I noticed it by running the in-tree script at scripts/checkkconfigsymbols.py on the commit, like so: './scripts/checkkconfigsymbols.py -c 34a730aa74c7' As Greg suggested the whole code could be dropped, this might not be too relevant, but I wanted to let you know in any case. Best regards, Andreas smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] regmap: make LZO cache optional
Hi Jonas, I noticed your patch 'regmap: make LZO cache optional' as it recently showed up in linux-next. In your patch, you modify drivers/base/regmap/regcache.c by adding an #if IS_ENABLED() statement. However, this statement contains a spelling error, as it references REGCHACHE_COMPRESSED instead of REGCACHE_COMPRESSED (note the extra H). I noticed it by running the in-tree script at scripts/checkkconfigsymbols.py on the commit, like so: './scripts/checkkconfigsymbols.py -c 34a730aa74c7' As Greg suggested the whole code could be dropped, this might not be too relevant, but I wanted to let you know in any case. Best regards, Andreas smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v3] cpufreq: brcmstb-cpufreq: CPUfreq driver for older Broadcom STB SoCs
Forgot to CC linux-kernel and linux-pm in my first message. On 11/28/2016 10:28, Andreas Ziegler wrote: > Hi Markus, > > your patch "cpufreq: brcmstb-cpufreq: CPUfreq driver for older Broadcom STB > SoCs" showed up in linux-next today, and I noticed a small error in it. > > In drivers/cpufreq/brcmstb-cpufreq.c, line 214 there is an IS_ENABLED() > statement with CONFIG_ARM_BRCM_AVS_CPUFREQ. This symbol, however, does not > exist > - making the if condition always evaluate to false. Did you mean to use > CONFIG_ARM_BRCMSTB_AVS_CPUFREQ (note the STB after BRCM)? > > I noticed this mistake by running 'scripts/checkkconfigsymbols -f --force -f > next-20161124..next-20161128', which essentialy diffs the last two > linux-next releases and looks for undefined/unknown Kconfig symbols. > You can also run the script on single commits with -c to test them. > > Best regards, > > Andreas >
Re: [PATCH v3] cpufreq: brcmstb-cpufreq: CPUfreq driver for older Broadcom STB SoCs
Forgot to CC linux-kernel and linux-pm in my first message. On 11/28/2016 10:28, Andreas Ziegler wrote: > Hi Markus, > > your patch "cpufreq: brcmstb-cpufreq: CPUfreq driver for older Broadcom STB > SoCs" showed up in linux-next today, and I noticed a small error in it. > > In drivers/cpufreq/brcmstb-cpufreq.c, line 214 there is an IS_ENABLED() > statement with CONFIG_ARM_BRCM_AVS_CPUFREQ. This symbol, however, does not > exist > - making the if condition always evaluate to false. Did you mean to use > CONFIG_ARM_BRCMSTB_AVS_CPUFREQ (note the STB after BRCM)? > > I noticed this mistake by running 'scripts/checkkconfigsymbols -f --force -f > next-20161124..next-20161128', which essentialy diffs the last two > linux-next releases and looks for undefined/unknown Kconfig symbols. > You can also run the script on single commits with -c to test them. > > Best regards, > > Andreas >
Re: [PATCH v3] module: extend 'rodata=off' boot cmdline parameter to module mappings
Hi Akashi, your patch "module: extend 'rodata=off' boot cmdline parameter to module mappings" showed up in linux-next today, and I noticed a small error in it. The first modified #ifdef is fine, the second one, however, has a spelling mistake in it: the CONFIG_ variable should be CONFIG_DEBUG_SET_MODULE_RONX instead of CONFIG_SET_MODULE_RONX (note the missing DEBUG). I noticed it by running 'scripts/checkkconfigsymbols -f --force -f next-20161124..next-20161128', which is essentialy diffing the last two linux-next releases and looks for undefined/unknown Kconfig symbols. You can also run the script on single commits with -c to test them. Best regards, Andreas
Re: [PATCH v3] module: extend 'rodata=off' boot cmdline parameter to module mappings
Hi Akashi, your patch "module: extend 'rodata=off' boot cmdline parameter to module mappings" showed up in linux-next today, and I noticed a small error in it. The first modified #ifdef is fine, the second one, however, has a spelling mistake in it: the CONFIG_ variable should be CONFIG_DEBUG_SET_MODULE_RONX instead of CONFIG_SET_MODULE_RONX (note the missing DEBUG). I noticed it by running 'scripts/checkkconfigsymbols -f --force -f next-20161124..next-20161128', which is essentialy diffing the last two linux-next releases and looks for undefined/unknown Kconfig symbols. You can also run the script on single commits with -c to test them. Best regards, Andreas
[PATCH] printk: Remove unnecessary #ifdef CONFIG_PRINTK
In commit 874f9c7da9a4 ("printk: create pr_ functions") in linux-next, new pr_level defines were added to printk.c. These defines are guarded by an #ifdef CONFIG_PRINTK - however, there is already a surrounding #ifdef CONFIG_PRINTK starting a lot earlier in line 249 which means the newly introduced #ifdef is unnecessary. Let's remove it to avoid confusion. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- kernel/printk/printk.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index a5ef95c..a37fc8c 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -1930,7 +1930,6 @@ asmlinkage int printk_emit(int facility, int level, } EXPORT_SYMBOL(printk_emit); -#ifdef CONFIG_PRINTK #define define_pr_level(func, loglevel)\ asmlinkage __visible void func(const char *fmt, ...) \ { \ @@ -1949,7 +1948,6 @@ define_pr_level(__pr_err, LOGLEVEL_ERR); define_pr_level(__pr_warn, LOGLEVEL_WARNING); define_pr_level(__pr_notice, LOGLEVEL_NOTICE); define_pr_level(__pr_info, LOGLEVEL_INFO); -#endif int vprintk_default(int level, const char *fmt, va_list args) { -- 1.9.1
[PATCH] printk: Remove unnecessary #ifdef CONFIG_PRINTK
In commit 874f9c7da9a4 ("printk: create pr_ functions") in linux-next, new pr_level defines were added to printk.c. These defines are guarded by an #ifdef CONFIG_PRINTK - however, there is already a surrounding #ifdef CONFIG_PRINTK starting a lot earlier in line 249 which means the newly introduced #ifdef is unnecessary. Let's remove it to avoid confusion. Signed-off-by: Andreas Ziegler --- kernel/printk/printk.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index a5ef95c..a37fc8c 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -1930,7 +1930,6 @@ asmlinkage int printk_emit(int facility, int level, } EXPORT_SYMBOL(printk_emit); -#ifdef CONFIG_PRINTK #define define_pr_level(func, loglevel)\ asmlinkage __visible void func(const char *fmt, ...) \ { \ @@ -1949,7 +1948,6 @@ define_pr_level(__pr_err, LOGLEVEL_ERR); define_pr_level(__pr_warn, LOGLEVEL_WARNING); define_pr_level(__pr_notice, LOGLEVEL_NOTICE); define_pr_level(__pr_info, LOGLEVEL_INFO); -#endif int vprintk_default(int level, const char *fmt, va_list args) { -- 1.9.1
Re: [PATCHv2 0/7] eBPF JIT for PPC64
Hi Naveen, this patchset makes a change to arch/powerpc/net/Makefile in order to only compile the previously existing bpf_jit_comp.c if !CONFIG_PPC64, and use bpf_jit_comp64.c if CONFIG_PPC64 is enabled. Inside arch/powerpc/net/bpf_jit_comp.c, however, there is still an #ifdef CONFIG_PPC64 block at line 667 (linux-next of today, i.e., next-20160630): #ifdef CONFIG_PPC64 /* Function descriptor nastiness: Address + TOC */ ((u64 *)image)[0] = (u64)code_base; ((u64 *)image)[1] = local_paca->kernel_toc; #endif >From my understanding of the code, this #ifdef can now be removed, as there is no way the file could be compiled with CONFIG_PPC64 enabled. Is this correct? Best regards, Andreas
Re: [PATCHv2 0/7] eBPF JIT for PPC64
Hi Naveen, this patchset makes a change to arch/powerpc/net/Makefile in order to only compile the previously existing bpf_jit_comp.c if !CONFIG_PPC64, and use bpf_jit_comp64.c if CONFIG_PPC64 is enabled. Inside arch/powerpc/net/bpf_jit_comp.c, however, there is still an #ifdef CONFIG_PPC64 block at line 667 (linux-next of today, i.e., next-20160630): #ifdef CONFIG_PPC64 /* Function descriptor nastiness: Address + TOC */ ((u64 *)image)[0] = (u64)code_base; ((u64 *)image)[1] = local_paca->kernel_toc; #endif >From my understanding of the code, this #ifdef can now be removed, as there is no way the file could be compiled with CONFIG_PPC64 enabled. Is this correct? Best regards, Andreas
select on non-existing Kconfig option CRC32C
Hi Hendrik, your patch "s390/crc32-vx: add crypto API module for optimized CRC-32 algorithms" showed up in linux-next today (next-20160615) as commit 364148e0b195. The patch defines the Kconfig option CRYPTO_CRC32_S390 which 'select's CRC32C. However, this should probably have been CRYPTO_CRC32C, as CRC32C does not exist. Should I prepare a trivial patch to fix this up or would you like to do that on your side? I found this issue by comparing yesterday's tree and today's tree using 'scripts/checkkconfigsymbols -f -d next-20160614..next-20160615'. Best regards, Andreas
select on non-existing Kconfig option CRC32C
Hi Hendrik, your patch "s390/crc32-vx: add crypto API module for optimized CRC-32 algorithms" showed up in linux-next today (next-20160615) as commit 364148e0b195. The patch defines the Kconfig option CRYPTO_CRC32_S390 which 'select's CRC32C. However, this should probably have been CRYPTO_CRC32C, as CRC32C does not exist. Should I prepare a trivial patch to fix this up or would you like to do that on your side? I found this issue by comparing yesterday's tree and today's tree using 'scripts/checkkconfigsymbols -f -d next-20160614..next-20160615'. Best regards, Andreas
[PATCH] drivers/net/fsl_ucc: Do not prefix header guard with CONFIG_
The CONFIG_ prefix should only be used for options which can be configured through Kconfig and not for guarding headers. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- drivers/net/wan/fsl_ucc_hdlc.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h index 525786a..881ecde 100644 --- a/drivers/net/wan/fsl_ucc_hdlc.h +++ b/drivers/net/wan/fsl_ucc_hdlc.h @@ -8,8 +8,8 @@ * option) any later version. */ -#ifndef CONFIG_UCC_HDLC_H -#define CONFIG_UCC_HDLC_H +#ifndef _UCC_HDLC_H_ +#define _UCC_HDLC_H_ #include #include -- 1.9.1
[PATCH] drivers/net/fsl_ucc: Do not prefix header guard with CONFIG_
The CONFIG_ prefix should only be used for options which can be configured through Kconfig and not for guarding headers. Signed-off-by: Andreas Ziegler --- drivers/net/wan/fsl_ucc_hdlc.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h index 525786a..881ecde 100644 --- a/drivers/net/wan/fsl_ucc_hdlc.h +++ b/drivers/net/wan/fsl_ucc_hdlc.h @@ -8,8 +8,8 @@ * option) any later version. */ -#ifndef CONFIG_UCC_HDLC_H -#define CONFIG_UCC_HDLC_H +#ifndef _UCC_HDLC_H_ +#define _UCC_HDLC_H_ #include #include -- 1.9.1
[PATCH] fsl/qe: Do not prefix header guard with CONFIG_
The CONFIG_ prefix should only be used for options which can be configured through Kconfig and not for guarding headers. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- include/soc/fsl/qe/qe_tdm.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/soc/fsl/qe/qe_tdm.h b/include/soc/fsl/qe/qe_tdm.h index 4c91498..a1664b6 100644 --- a/include/soc/fsl/qe/qe_tdm.h +++ b/include/soc/fsl/qe/qe_tdm.h @@ -11,8 +11,8 @@ * option) any later version */ -#ifndef CONFIG_QE_TDM_H -#define CONFIG_QE_TDM_H +#ifndef _QE_TDM_H_ +#define _QE_TDM_H_ #include #include -- 1.9.1
[PATCH] fsl/qe: Do not prefix header guard with CONFIG_
The CONFIG_ prefix should only be used for options which can be configured through Kconfig and not for guarding headers. Signed-off-by: Andreas Ziegler --- include/soc/fsl/qe/qe_tdm.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/soc/fsl/qe/qe_tdm.h b/include/soc/fsl/qe/qe_tdm.h index 4c91498..a1664b6 100644 --- a/include/soc/fsl/qe/qe_tdm.h +++ b/include/soc/fsl/qe/qe_tdm.h @@ -11,8 +11,8 @@ * option) any later version */ -#ifndef CONFIG_QE_TDM_H -#define CONFIG_QE_TDM_H +#ifndef _QE_TDM_H_ +#define _QE_TDM_H_ #include #include -- 1.9.1
[PATCH] checkkconfigsymbols.py: Fix typo in help message
Fix a typo in the help message for the -d parameter by removing one 'm'. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- scripts/checkkconfigsymbols.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py index d8f6c09..df643f6 100755 --- a/scripts/checkkconfigsymbols.py +++ b/scripts/checkkconfigsymbols.py @@ -89,7 +89,7 @@ def parse_options(): if opts.diff and not re.match(r"^[\w\-\.]+\.\.[\w\-\.]+$", opts.diff): sys.exit("Please specify valid input in the following format: " - "\'commmit1..commit2\'") + "\'commit1..commit2\'") if opts.commit or opts.diff: if not opts.force and tree_is_dirty(): -- 1.9.1
[PATCH] checkkconfigsymbols.py: Fix typo in help message
Fix a typo in the help message for the -d parameter by removing one 'm'. Signed-off-by: Andreas Ziegler --- scripts/checkkconfigsymbols.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py index d8f6c09..df643f6 100755 --- a/scripts/checkkconfigsymbols.py +++ b/scripts/checkkconfigsymbols.py @@ -89,7 +89,7 @@ def parse_options(): if opts.diff and not re.match(r"^[\w\-\.]+\.\.[\w\-\.]+$", opts.diff): sys.exit("Please specify valid input in the following format: " - "\'commmit1..commit2\'") + "\'commit1..commit2\'") if opts.commit or opts.diff: if not opts.force and tree_is_dirty(): -- 1.9.1
[PATCH] PCI: cleanup pci/pcie/Kconfig
This change cleans up style issues in drivers/pci/pcie/Kconfig, in particular all indentation is now done using tabs, not spaces, and the definition of PCIEASPM_DEBUG is now separated from the definition of PCIEASPM with a newline. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- drivers/pci/pcie/Kconfig | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig index e294713..72db7f4 100644 --- a/drivers/pci/pcie/Kconfig +++ b/drivers/pci/pcie/Kconfig @@ -44,6 +44,7 @@ config PCIEASPM /sys/module/pcie_aspm/parameters/policy When in doubt, say Y. + config PCIEASPM_DEBUG bool "Debug PCI Express ASPM" depends on PCIEASPM @@ -58,20 +59,20 @@ choice depends on PCIEASPM config PCIEASPM_DEFAULT -bool "BIOS default" + bool "BIOS default" depends on PCIEASPM help Use the BIOS defaults for PCI Express ASPM. config PCIEASPM_POWERSAVE -bool "Powersave" + bool "Powersave" depends on PCIEASPM help Enable PCI Express ASPM L0s and L1 where possible, even if the BIOS did not. config PCIEASPM_PERFORMANCE -bool "Performance" + bool "Performance" depends on PCIEASPM help Disable PCI Express ASPM L0s and L1, even if the BIOS enabled them. -- 1.9.1
[PATCH] PCI: cleanup pci/pcie/Kconfig
This change cleans up style issues in drivers/pci/pcie/Kconfig, in particular all indentation is now done using tabs, not spaces, and the definition of PCIEASPM_DEBUG is now separated from the definition of PCIEASPM with a newline. Signed-off-by: Andreas Ziegler --- drivers/pci/pcie/Kconfig | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig index e294713..72db7f4 100644 --- a/drivers/pci/pcie/Kconfig +++ b/drivers/pci/pcie/Kconfig @@ -44,6 +44,7 @@ config PCIEASPM /sys/module/pcie_aspm/parameters/policy When in doubt, say Y. + config PCIEASPM_DEBUG bool "Debug PCI Express ASPM" depends on PCIEASPM @@ -58,20 +59,20 @@ choice depends on PCIEASPM config PCIEASPM_DEFAULT -bool "BIOS default" + bool "BIOS default" depends on PCIEASPM help Use the BIOS defaults for PCI Express ASPM. config PCIEASPM_POWERSAVE -bool "Powersave" + bool "Powersave" depends on PCIEASPM help Enable PCI Express ASPM L0s and L1 where possible, even if the BIOS did not. config PCIEASPM_PERFORMANCE -bool "Performance" + bool "Performance" depends on PCIEASPM help Disable PCI Express ASPM L0s and L1, even if the BIOS enabled them. -- 1.9.1
[PATCH] ARC: Don't source drivers/pci/pcie/Kconfig ourselves
Commit 5f8fc43217a0 ("PCI: Include pci/pcie/Kconfig directly from pci/Kconfig") in linux-next changed drivers/pci/Kconfig to include drivers/pci/pcie/Kconfig itself, so that architectures do not need to source both files themselves. ARC just recently gained PCI support through commit 6b3fb77998dd ("ARC: Add PCI support"), but this change was based on the old behaviour of the Kconfig files. This makes Kconfig now spit out the following warnings: drivers/pci/pcie/Kconfig:61:warning: choice value used outside its choice group drivers/pci/pcie/Kconfig:67:warning: choice value used outside its choice group drivers/pci/pcie/Kconfig:74:warning: choice value used outside its choice group This change updates the Kconfig file for ARC, dropping the now unnecessary 'source' statement, which makes the warning disappear. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- arch/arc/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig index 69d05f5..282efec 100644 --- a/arch/arc/Kconfig +++ b/arch/arc/Kconfig @@ -592,7 +592,6 @@ config PCI_SYSCALL def_bool PCI source "drivers/pci/Kconfig" -source "drivers/pci/pcie/Kconfig" endmenu -- 1.9.1
[PATCH] ARC: Don't source drivers/pci/pcie/Kconfig ourselves
Commit 5f8fc43217a0 ("PCI: Include pci/pcie/Kconfig directly from pci/Kconfig") in linux-next changed drivers/pci/Kconfig to include drivers/pci/pcie/Kconfig itself, so that architectures do not need to source both files themselves. ARC just recently gained PCI support through commit 6b3fb77998dd ("ARC: Add PCI support"), but this change was based on the old behaviour of the Kconfig files. This makes Kconfig now spit out the following warnings: drivers/pci/pcie/Kconfig:61:warning: choice value used outside its choice group drivers/pci/pcie/Kconfig:67:warning: choice value used outside its choice group drivers/pci/pcie/Kconfig:74:warning: choice value used outside its choice group This change updates the Kconfig file for ARC, dropping the now unnecessary 'source' statement, which makes the warning disappear. Signed-off-by: Andreas Ziegler --- arch/arc/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig index 69d05f5..282efec 100644 --- a/arch/arc/Kconfig +++ b/arch/arc/Kconfig @@ -592,7 +592,6 @@ config PCI_SYSCALL def_bool PCI source "drivers/pci/Kconfig" -source "drivers/pci/pcie/Kconfig" endmenu -- 1.9.1
[PATCH] security: integrity: Remove select to deleted option PUBLIC_KEY_ALGO_RSA
Commit d43de6c780a8 ("akcipher: Move the RSA DER encoding check to the crypto layer") removed the Kconfig option PUBLIC_KEY_ALGO_RSA, but forgot to remove a 'select' to this option in the definition of INTEGRITY_ASYMMETRIC_KEYS. Let's remove the select, as it's ineffective now. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- security/integrity/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 979be65..da95658 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -35,7 +35,6 @@ config INTEGRITY_ASYMMETRIC_KEYS default n select ASYMMETRIC_KEY_TYPE select ASYMMETRIC_PUBLIC_KEY_SUBTYPE -select PUBLIC_KEY_ALGO_RSA select CRYPTO_RSA select X509_CERTIFICATE_PARSER help -- 1.9.1
[PATCH] security: integrity: Remove select to deleted option PUBLIC_KEY_ALGO_RSA
Commit d43de6c780a8 ("akcipher: Move the RSA DER encoding check to the crypto layer") removed the Kconfig option PUBLIC_KEY_ALGO_RSA, but forgot to remove a 'select' to this option in the definition of INTEGRITY_ASYMMETRIC_KEYS. Let's remove the select, as it's ineffective now. Signed-off-by: Andreas Ziegler --- security/integrity/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 979be65..da95658 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -35,7 +35,6 @@ config INTEGRITY_ASYMMETRIC_KEYS default n select ASYMMETRIC_KEY_TYPE select ASYMMETRIC_PUBLIC_KEY_SUBTYPE -select PUBLIC_KEY_ALGO_RSA select CRYPTO_RSA select X509_CERTIFICATE_PARSER help -- 1.9.1
Left-over select to PUBLIC_KEY_ALGO_RSA
Hi David, your patch "akcipher: Move the RSA DER encoding check to the crypto layer" showed up in linux-next today as commit d43de6c780a8 (that is, next-20160304). It removes the CONFIG_PUBLIC_KEY_ALGO_RSA option, but does leave one reference to it in place inside security/integrity/Kconfig, in the definition of CONFIG_INTEGRITY_ASYMMETRIC_KEYS. As the corresponding option is gone, the select statement can safely be removed. Should I prepare a simple patch for that? I detected this by using scripts/checkkconfigsymbols on today's and yesterday's linux-next trees (i.e., "./scripts/checkkconfigsymbols.py -d next-20160303..next-20160304"). Best regards, Andreas
Left-over select to PUBLIC_KEY_ALGO_RSA
Hi David, your patch "akcipher: Move the RSA DER encoding check to the crypto layer" showed up in linux-next today as commit d43de6c780a8 (that is, next-20160304). It removes the CONFIG_PUBLIC_KEY_ALGO_RSA option, but does leave one reference to it in place inside security/integrity/Kconfig, in the definition of CONFIG_INTEGRITY_ASYMMETRIC_KEYS. As the corresponding option is gone, the select statement can safely be removed. Should I prepare a simple patch for that? I detected this by using scripts/checkkconfigsymbols on today's and yesterday's linux-next trees (i.e., "./scripts/checkkconfigsymbols.py -d next-20160303..next-20160304"). Best regards, Andreas
[PATCH] security: Remove select to deleted KEYS_DEBUG_PROC_KEYS option
Commit f4dc37785e9b ("integrity: define '.evm' as a builtin 'trusted' keyring") introduces the INTEGRITY_TRUSTED_KEYRING Kconfig option. This option 'select's KEYS_DEBUG_PROC_KEYS which has been removed earlier by commit dabd39cc2fb1 ("KEYS: Make /proc/keys unconditional if CONFIG_KEYS=y"). This patch removes the inoperative select. Signed-off-by: Andreas Ziegler --- security/integrity/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 21d7568..7543398 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -45,7 +45,6 @@ config INTEGRITY_TRUSTED_KEYRING bool "Require all keys on the integrity keyrings be signed" depends on SYSTEM_TRUSTED_KEYRING depends on INTEGRITY_ASYMMETRIC_KEYS - select KEYS_DEBUG_PROC_KEYS default y help This option requires that all keys added to the .ima and -- 1.9.1
'select' on deleted KEYS_DEBUG_PROC_KEYS option
Hi Dmitry, your commit f4dc37785e9b ("integrity: define '.evm' as a builtin 'trusted' keyring") introduces the INTEGRITY_TRUSTED_KEYRING Kconfig option, which has a 'select' to KEYS_DEBUG_PROC_KEYS. This option, however, has been removed over a year ago by commit dabd39cc2fb1 ("KEYS: Make /proc/keys unconditional if CONFIG_KEYS=y") and -- besides the 'select' mentioned above -- only occurs inside (outdated) defconfigs. Should I prepare a simple patch removing the select? Best regards, Andreas
Re: [PATCH] cgroup: Fix misspelling of CONFIG_SOCK_CGROUP_DATA in comments
On 01/26/2016 16:08, Rosen, Rami wrote: > Hi, > > --- a/include/linux/cgroup.h > +++ b/include/linux/cgroup.h > @@ -604,11 +604,11 @@ static inline struct cgroup *sock_cgroup_ptr(struct > sock_cgroup_data *skcd) > #endif > } > > In this occasion, seems that maybe something else is also missing: > Shouldn't it be hereafter : +#else /* !CONFIG_SOCK_CGROUP_DATA */ > instead ? > > -#else/* CONFIG_CGROUP_DATA */ > +#else/* CONFIG_SOCK_CGROUP_DATA */ It seems that there is no real consensus among the developers for that particular case: ziegler@box:~/linux$ git grep "#else \/\* \!CONFIG_" | wc -l 327 ziegler@box:~/linux$ git grep "#else \/\* CONFIG_" | wc -l 564 I don't mind changing it, I'm just not sure if that's what we want. Regards, Andreas
Use of deleted Kconfig option B43_PCMCIA
Hi Rafał, your commit 399500da18f7 ("ssb: pick PCMCIA host code support from b43 driver") removed the Kconfig option B43_PCMCIA from Kconfig, but left one reference to it in b43/main.c, inside the b43_print_driverinfo() function. Can this #ifdef be removed, should it be replaced with an #ifdef using CONFIG_SSB_PCMCIAHOST instead or should we do something entirely different? Best regards, Andreas
[PATCH] cgroup: Fix misspelling of CONFIG_SOCK_CGROUP_DATA in comments
Commit bd1060a1d671 ("sock, cgroup: add sock->sk_cgroup") added new code guarded by an #ifdef CONFIG_SOCK_CGROUP_DATA to include/linux/cgroup.h. In the comments for the corresponding #else and #endifs, however, the option is misspelled as CONFIG_CGROUP_DATA. Fix those comments. Signed-off-by: Andreas Ziegler --- include/linux/cgroup.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 2162dca..48ce12f 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -604,11 +604,11 @@ static inline struct cgroup *sock_cgroup_ptr(struct sock_cgroup_data *skcd) #endif } -#else /* CONFIG_CGROUP_DATA */ +#else /* CONFIG_SOCK_CGROUP_DATA */ static inline void cgroup_sk_alloc(struct sock_cgroup_data *skcd) {} static inline void cgroup_sk_free(struct sock_cgroup_data *skcd) {} -#endif /* CONFIG_CGROUP_DATA */ +#endif /* CONFIG_SOCK_CGROUP_DATA */ #endif /* _LINUX_CGROUP_H */ -- 1.9.1
[PATCH] cgroup: Fix misspelling of CONFIG_SOCK_CGROUP_DATA in comments
Commit bd1060a1d671 ("sock, cgroup: add sock->sk_cgroup") added new code guarded by an #ifdef CONFIG_SOCK_CGROUP_DATA to include/linux/cgroup.h. In the comments for the corresponding #else and #endifs, however, the option is misspelled as CONFIG_CGROUP_DATA. Fix those comments. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- include/linux/cgroup.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 2162dca..48ce12f 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -604,11 +604,11 @@ static inline struct cgroup *sock_cgroup_ptr(struct sock_cgroup_data *skcd) #endif } -#else /* CONFIG_CGROUP_DATA */ +#else /* CONFIG_SOCK_CGROUP_DATA */ static inline void cgroup_sk_alloc(struct sock_cgroup_data *skcd) {} static inline void cgroup_sk_free(struct sock_cgroup_data *skcd) {} -#endif /* CONFIG_CGROUP_DATA */ +#endif /* CONFIG_SOCK_CGROUP_DATA */ #endif /* _LINUX_CGROUP_H */ -- 1.9.1
Use of deleted Kconfig option B43_PCMCIA
Hi Rafał, your commit 399500da18f7 ("ssb: pick PCMCIA host code support from b43 driver") removed the Kconfig option B43_PCMCIA from Kconfig, but left one reference to it in b43/main.c, inside the b43_print_driverinfo() function. Can this #ifdef be removed, should it be replaced with an #ifdef using CONFIG_SSB_PCMCIAHOST instead or should we do something entirely different? Best regards, Andreas
Re: [PATCH] cgroup: Fix misspelling of CONFIG_SOCK_CGROUP_DATA in comments
On 01/26/2016 16:08, Rosen, Rami wrote: > Hi, > > --- a/include/linux/cgroup.h > +++ b/include/linux/cgroup.h > @@ -604,11 +604,11 @@ static inline struct cgroup *sock_cgroup_ptr(struct > sock_cgroup_data *skcd) > #endif > } > > In this occasion, seems that maybe something else is also missing: > Shouldn't it be hereafter : +#else /* !CONFIG_SOCK_CGROUP_DATA */ > instead ? > > -#else/* CONFIG_CGROUP_DATA */ > +#else/* CONFIG_SOCK_CGROUP_DATA */ It seems that there is no real consensus among the developers for that particular case: ziegler@box:~/linux$ git grep "#else \/\* \!CONFIG_" | wc -l 327 ziegler@box:~/linux$ git grep "#else \/\* CONFIG_" | wc -l 564 I don't mind changing it, I'm just not sure if that's what we want. Regards, Andreas
'select' on deleted KEYS_DEBUG_PROC_KEYS option
Hi Dmitry, your commit f4dc37785e9b ("integrity: define '.evm' as a builtin 'trusted' keyring") introduces the INTEGRITY_TRUSTED_KEYRING Kconfig option, which has a 'select' to KEYS_DEBUG_PROC_KEYS. This option, however, has been removed over a year ago by commit dabd39cc2fb1 ("KEYS: Make /proc/keys unconditional if CONFIG_KEYS=y") and -- besides the 'select' mentioned above -- only occurs inside (outdated) defconfigs. Should I prepare a simple patch removing the select? Best regards, Andreas
[PATCH] security: Remove select to deleted KEYS_DEBUG_PROC_KEYS option
Commit f4dc37785e9b ("integrity: define '.evm' as a builtin 'trusted' keyring") introduces the INTEGRITY_TRUSTED_KEYRING Kconfig option. This option 'select's KEYS_DEBUG_PROC_KEYS which has been removed earlier by commit dabd39cc2fb1 ("KEYS: Make /proc/keys unconditional if CONFIG_KEYS=y"). This patch removes the inoperative select. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- security/integrity/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 21d7568..7543398 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -45,7 +45,6 @@ config INTEGRITY_TRUSTED_KEYRING bool "Require all keys on the integrity keyrings be signed" depends on SYSTEM_TRUSTED_KEYRING depends on INTEGRITY_ASYMMETRIC_KEYS - select KEYS_DEBUG_PROC_KEYS default y help This option requires that all keys added to the .ima and -- 1.9.1
[PATCH v2] drm/i915: Remove select to deleted STOP_MACHINE from Kconfig
Commit 5bab6f60cb4d ("drm/i915: Serialise updates to GGTT with access through GGTT on Braswell") depended upon a working stop_machine() and so forced the selection of STOP_MACHINE. However, commit 86fffe4a61dd ("kernel: remove stop_machine() Kconfig dependency") removed the option STOP_MACHINE from init/Kconfig and ensured that stop_machine() universally works. Due to the order in which the patches were applied, removing the select from DRM_I915 got lost during merging. Remove the now obsolete select statement. Signed-off-by: Andreas Ziegler Reviewed-by: Chris Wilson --- Changes to v1: - Tell the story right, suggested by Chris Wilson drivers/gpu/drm/i915/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index fcd77b2..051eab3 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -10,7 +10,6 @@ config DRM_I915 # the shmem_readpage() which depends upon tmpfs select SHMEM select TMPFS - select STOP_MACHINE select DRM_KMS_HELPER select DRM_PANEL select DRM_MIPI_DSI -- 1.9.1
[PATCH] drm/i915: Remove select to deleted STOP_MACHINE from Kconfig
Commit 86fffe4a61dd ("kernel: remove stop_machine() Kconfig dependency") removed the option STOP_MACHINE from init/Kconfig, but missed a select to this option from the DRM_I915 option. Remove this now obsolete select statement. Signed-off-by: Andreas Ziegler --- drivers/gpu/drm/i915/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index fcd77b2..051eab3 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -10,7 +10,6 @@ config DRM_I915 # the shmem_readpage() which depends upon tmpfs select SHMEM select TMPFS - select STOP_MACHINE select DRM_KMS_HELPER select DRM_PANEL select DRM_MIPI_DSI -- 1.9.1
Select on now-gone Kconfig option STOP_MACHINE
Hi Chris, your commit 5bab6f60cb4d ("drm/i915: Serialise updates to GGTT with access through GGTT on Braswell") added a select to STOP_MACHINE to the DRM_I915 Kconfig option. In 4.5-rc1, your commit 86fffe4a61dd ("kernel: remove stop_machine() Kconfig dependency") then removed STOP_MACHINE entirely, but didn't delete the select from drivers/gpu/drm/i915/Kconfig. Should I prepare a simple patch which removes the select? Best regards, Andreas
[PATCH] mm: Fix two typos in comments for to_vmem_altmap()
Commit 4b94ffdc4163 ("x86, mm: introduce vmem_altmap to augment vmemmap_populate()"), introduced the to_vmem_altmap() function. The comments in this function contain two typos (one misspelling of the Kconfig option CONFIG_SPARSEMEM_VMEMMAP, and one missing letter 'n'), let's fix them up. Signed-off-by: Andreas Ziegler --- kernel/memremap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/memremap.c b/kernel/memremap.c index e517a16..2e84aa8 100644 --- a/kernel/memremap.c +++ b/kernel/memremap.c @@ -377,7 +377,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start) /* * 'memmap_start' is the virtual address for the first "struct * page" in this range of the vmemmap array. In the case of -* CONFIG_SPARSE_VMEMMAP a page_to_pfn conversion is simple +* CONFIG_SPARSEMEM_VMEMMAP a page_to_pfn conversion is simple * pointer arithmetic, so we can perform this to_vmem_altmap() * conversion without concern for the initialization state of * the struct page fields. @@ -386,7 +386,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start) struct dev_pagemap *pgmap; /* -* Uncoditionally retrieve a dev_pagemap associated with the +* Unconditionally retrieve a dev_pagemap associated with the * given physical address, this is only for use in the * arch_{add|remove}_memory() for setting up and tearing down * the memmap. -- 1.9.1
[PATCH] mm: Fix two typos in comments for to_vmem_altmap()
Commit 4b94ffdc4163 ("x86, mm: introduce vmem_altmap to augment vmemmap_populate()"), introduced the to_vmem_altmap() function. The comments in this function contain two typos (one misspelling of the Kconfig option CONFIG_SPARSEMEM_VMEMMAP, and one missing letter 'n'), let's fix them up. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- kernel/memremap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/memremap.c b/kernel/memremap.c index e517a16..2e84aa8 100644 --- a/kernel/memremap.c +++ b/kernel/memremap.c @@ -377,7 +377,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start) /* * 'memmap_start' is the virtual address for the first "struct * page" in this range of the vmemmap array. In the case of -* CONFIG_SPARSE_VMEMMAP a page_to_pfn conversion is simple +* CONFIG_SPARSEMEM_VMEMMAP a page_to_pfn conversion is simple * pointer arithmetic, so we can perform this to_vmem_altmap() * conversion without concern for the initialization state of * the struct page fields. @@ -386,7 +386,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start) struct dev_pagemap *pgmap; /* -* Uncoditionally retrieve a dev_pagemap associated with the +* Unconditionally retrieve a dev_pagemap associated with the * given physical address, this is only for use in the * arch_{add|remove}_memory() for setting up and tearing down * the memmap. -- 1.9.1
[PATCH] drm/i915: Remove select to deleted STOP_MACHINE from Kconfig
Commit 86fffe4a61dd ("kernel: remove stop_machine() Kconfig dependency") removed the option STOP_MACHINE from init/Kconfig, but missed a select to this option from the DRM_I915 option. Remove this now obsolete select statement. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> --- drivers/gpu/drm/i915/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index fcd77b2..051eab3 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -10,7 +10,6 @@ config DRM_I915 # the shmem_readpage() which depends upon tmpfs select SHMEM select TMPFS - select STOP_MACHINE select DRM_KMS_HELPER select DRM_PANEL select DRM_MIPI_DSI -- 1.9.1
Select on now-gone Kconfig option STOP_MACHINE
Hi Chris, your commit 5bab6f60cb4d ("drm/i915: Serialise updates to GGTT with access through GGTT on Braswell") added a select to STOP_MACHINE to the DRM_I915 Kconfig option. In 4.5-rc1, your commit 86fffe4a61dd ("kernel: remove stop_machine() Kconfig dependency") then removed STOP_MACHINE entirely, but didn't delete the select from drivers/gpu/drm/i915/Kconfig. Should I prepare a simple patch which removes the select? Best regards, Andreas
[PATCH v2] drm/i915: Remove select to deleted STOP_MACHINE from Kconfig
Commit 5bab6f60cb4d ("drm/i915: Serialise updates to GGTT with access through GGTT on Braswell") depended upon a working stop_machine() and so forced the selection of STOP_MACHINE. However, commit 86fffe4a61dd ("kernel: remove stop_machine() Kconfig dependency") removed the option STOP_MACHINE from init/Kconfig and ensured that stop_machine() universally works. Due to the order in which the patches were applied, removing the select from DRM_I915 got lost during merging. Remove the now obsolete select statement. Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de> Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk> --- Changes to v1: - Tell the story right, suggested by Chris Wilson drivers/gpu/drm/i915/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index fcd77b2..051eab3 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -10,7 +10,6 @@ config DRM_I915 # the shmem_readpage() which depends upon tmpfs select SHMEM select TMPFS - select STOP_MACHINE select DRM_KMS_HELPER select DRM_PANEL select DRM_MIPI_DSI -- 1.9.1
Re: MIPS: BMIPS: Enable GZIP ramdisk and timed printks
Hi Florian, your patch "MIPS: BMIPS: Enable GZIP ramdisk and timed printks" showed up as commit fb9e5642aa9e in linux-next today (that is, next-20151112). I noticed it because we (a research group from Erlangen[0]) are running daily checks on linux-next. Your commit adds two entries to arch/mips/configs/bmips_stb_defconfig, but one of them has a typo (line 37): CONFIG_PRINK_TIME=y which should instead say (notice the missing 'T'): CONFIG_PRINTK_TIME=y Not sure how this got two Reviewed-by's, as this simple mistake could have been easily spotted by running scripts/checkkconfigsymbols.py on the patch. Regards, Andreas [0] https://cados.cs.fau.de Original patchwork: https://patchwork.linux-mips.org/patch/11307/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: MIPS: BMIPS: Enable GZIP ramdisk and timed printks
Hi Florian, your patch "MIPS: BMIPS: Enable GZIP ramdisk and timed printks" showed up as commit fb9e5642aa9e in linux-next today (that is, next-20151112). I noticed it because we (a research group from Erlangen[0]) are running daily checks on linux-next. Your commit adds two entries to arch/mips/configs/bmips_stb_defconfig, but one of them has a typo (line 37): CONFIG_PRINK_TIME=y which should instead say (notice the missing 'T'): CONFIG_PRINTK_TIME=y Not sure how this got two Reviewed-by's, as this simple mistake could have been easily spotted by running scripts/checkkconfigsymbols.py on the patch. Regards, Andreas [0] https://cados.cs.fau.de Original patchwork: https://patchwork.linux-mips.org/patch/11307/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0
Hi Jarkko, your patch "tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0" showed up as commit 399235dc6e95 in linux-next today (that is, next-20151020). I noticed it because we (a research group from Erlangen[0]) are running daily checks on linux-next. Your commit creates the following structure of #ifdef blocks in drivers/char/tpm/tpm_tis.c following line 1088: #ifdef CONFIG_ACPI ... #ifdef CONFIG_PNP ... #endif ... #endif Looking at the definition of CONFIG_ACPI at drivers/acpi/Kconfig, line 5, we see that ACPI unconditionally selects PNP, meaning that CONFIG_PNP is always enabled if CONFIG_ACPI has been enabled. Thus, the inner #ifdef statement can never evaluate to 'false' if the outer #ifdef evaluates to true (i.e., CONFIG_ACPI is enabled), and hence, the #ifdef is unnecessary. The same situation holds for the nested structure following line 1124, where the #ifdef CONFIG_PNP at line 1129 is unnecessary. Is this correct or did we miss something? Regards, Andreas [0] https://cados.cs.fau.de -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0
Hi Jarkko, your patch "tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0" showed up as commit 399235dc6e95 in linux-next today (that is, next-20151020). I noticed it because we (a research group from Erlangen[0]) are running daily checks on linux-next. Your commit creates the following structure of #ifdef blocks in drivers/char/tpm/tpm_tis.c following line 1088: #ifdef CONFIG_ACPI ... #ifdef CONFIG_PNP ... #endif ... #endif Looking at the definition of CONFIG_ACPI at drivers/acpi/Kconfig, line 5, we see that ACPI unconditionally selects PNP, meaning that CONFIG_PNP is always enabled if CONFIG_ACPI has been enabled. Thus, the inner #ifdef statement can never evaluate to 'false' if the outer #ifdef evaluates to true (i.e., CONFIG_ACPI is enabled), and hence, the #ifdef is unnecessary. The same situation holds for the nested structure following line 1124, where the #ifdef CONFIG_PNP at line 1129 is unnecessary. Is this correct or did we miss something? Regards, Andreas [0] https://cados.cs.fau.de -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: spi/bcm63xx: fix standard accessors and compile guard
Hi Jonas, your patch "spi/bcm63xx: fix standard accessors and compile guard" showed up as commit 682b5280bf00 in linux-next today (that is, next-20151013). I noticed it because we (a research group from Erlangen[0]) are running daily checks on linux-next. Your commit fixes two #ifdef statements in drivers/spi/spi-bcm63xx.c which involve CONFIG_CPU_BIG_ENDIAN. >From the Makefile at drivers/spi/Makefile (line 20), we can see that the file can only be built when CONFIG_SPI_BCM63XX is set. In the corresponding Kconfig file (drivers/spi/Kconfig, line 137), CONFIG_SPI_BCM63XX is defined to depend on CONFIG_BCM63XX. The latter is defined in arch/mips/Kconfig (line 199), and selects CONFIG_SYS_SUPPORTS_BIG_ENDIAN (but not CONFIG_SYS_SUPPORTS_LITTLE_ENDIAN). Finally, CONFIG_CPU_BIG_ENDIAN (at arch/mips/Kconfig, line 1136) depends on CONFIG_SYS_SUPPORTS_BIG_ENDIAN, which means that if the source file is to be compiled, CONFIG_CPU_BIG_ENDIAN is the only possible selection in the endianness choice. Hence, the #ifdefs are unnecessary and could possibly be removed. Is this correct, or am I missing something? Best regards, Andreas [0] https://cados.cs.fau.de -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: spi/bcm63xx: fix standard accessors and compile guard
Hi Jonas, your patch "spi/bcm63xx: fix standard accessors and compile guard" showed up as commit 682b5280bf00 in linux-next today (that is, next-20151013). I noticed it because we (a research group from Erlangen[0]) are running daily checks on linux-next. Your commit fixes two #ifdef statements in drivers/spi/spi-bcm63xx.c which involve CONFIG_CPU_BIG_ENDIAN. >From the Makefile at drivers/spi/Makefile (line 20), we can see that the file can only be built when CONFIG_SPI_BCM63XX is set. In the corresponding Kconfig file (drivers/spi/Kconfig, line 137), CONFIG_SPI_BCM63XX is defined to depend on CONFIG_BCM63XX. The latter is defined in arch/mips/Kconfig (line 199), and selects CONFIG_SYS_SUPPORTS_BIG_ENDIAN (but not CONFIG_SYS_SUPPORTS_LITTLE_ENDIAN). Finally, CONFIG_CPU_BIG_ENDIAN (at arch/mips/Kconfig, line 1136) depends on CONFIG_SYS_SUPPORTS_BIG_ENDIAN, which means that if the source file is to be compiled, CONFIG_CPU_BIG_ENDIAN is the only possible selection in the endianness choice. Hence, the #ifdefs are unnecessary and could possibly be removed. Is this correct, or am I missing something? Best regards, Andreas [0] https://cados.cs.fau.de -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Weird Kconfig condition (IPV6 [=n] || IPV6 [=n]=n)
Hi, On 09/14/2015 10:41, Mason wrote: > Hello, > > I did make menuconfig in v4.2 > > In the help for CONFIG_CRYPTO_HMAC, I have > > │ Selected by: IP_SCTP [=n] && NET [=y] && INET [=y] && (IPV6 [=n] || > IPV6 [=n]=n) || INET_AH [=n] && NET [=y] && INET [=y] || \ │ > │ INET_ESP [=n] && NET [=y] && INET [=y] || INET6_AH [=n] && NET [=y] && > INET [=y] && IPV6 [=n] || INET6_ESP [=n] && NET [=y] && INET [=y]\ │ > │ && IPV6 [=n] || SCTP_COOKIE_HMAC_MD5 [=n] && NET [=y] && IP_SCTP [=n] && > SCTP_COOKIE_HMAC_MD5 [=n] || SCTP_COOKIE_HMAC_SHA1 [=n] && \│ > │ NET [=y] && IP_SCTP [=n] && SCTP_COOKIE_HMAC_SHA1 [=n] || CIFS [=n] && > NETWORK_FILESYSTEMS [=y] && INET [=y] || TRUSTED_KEYS [=n] && \│ > │ KEYS [=y] && TCG_TPM [=n] || ENCRYPTED_KEYS [=n] && KEYS [=y] || IMA [=n] > && INTEGRITY [=n] || EVM [=n] && INTEGRITY [=n] || \│ > │ CRYPTO_DRBG_HMAC [=y] && CRYPTO [=y] && CRYPTO_DRBG_MENU [=m] || > CRYPTO_DEV_OMAP_SHAM [=n] && CRYPTO [=y] && CRYPTO_HW [=y] && \ > │ > │ ARCH_OMAP2PLUS [=n] || CRYPTO_DEV_QAT [=n] && CRYPTO [=y] && CRYPTO_HW > [=y] │ > > > What does (IPV6 [=n] || IPV6 [=n]=n) mean? > Is this a bug in my configuration? No, this comes from the definition of IP_SCTP in net/sctp/Kconfig. menuconfig IP_SCTP tristate "The SCTP Protocol" depends on INET depends on IPV6 || IPV6=n ^^ This weird-looking condition has the effect that: - if IPV6 is disabled, IP_SCTP can be anything ("y","m" or "n") - if IPV6 is enabled to "y", IP_SCTP can also be set to anything. - (and this is the reason why it's there:) if IPV6 is a module ("m"), IP_SCTP can only be "m" or "n". In a way, the "depends on" does not only reference the symbol, but also keeps track of the 'state' of the symbol it references, such that if the dependency (here: IPV6) is to be built as a loadable module only, the dependant feature can also only be built as a loadable module. I can't find the Documentation for that at the moment, but I'm sure that I read this at some point, maybe also only on a mailing list... Regards, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Weird Kconfig condition (IPV6 [=n] || IPV6 [=n]=n)
Hi, On 09/14/2015 10:41, Mason wrote: > Hello, > > I did make menuconfig in v4.2 > > In the help for CONFIG_CRYPTO_HMAC, I have > > │ Selected by: IP_SCTP [=n] && NET [=y] && INET [=y] && (IPV6 [=n] || > IPV6 [=n]=n) || INET_AH [=n] && NET [=y] && INET [=y] || \ │ > │ INET_ESP [=n] && NET [=y] && INET [=y] || INET6_AH [=n] && NET [=y] && > INET [=y] && IPV6 [=n] || INET6_ESP [=n] && NET [=y] && INET [=y]\ │ > │ && IPV6 [=n] || SCTP_COOKIE_HMAC_MD5 [=n] && NET [=y] && IP_SCTP [=n] && > SCTP_COOKIE_HMAC_MD5 [=n] || SCTP_COOKIE_HMAC_SHA1 [=n] && \│ > │ NET [=y] && IP_SCTP [=n] && SCTP_COOKIE_HMAC_SHA1 [=n] || CIFS [=n] && > NETWORK_FILESYSTEMS [=y] && INET [=y] || TRUSTED_KEYS [=n] && \│ > │ KEYS [=y] && TCG_TPM [=n] || ENCRYPTED_KEYS [=n] && KEYS [=y] || IMA [=n] > && INTEGRITY [=n] || EVM [=n] && INTEGRITY [=n] || \│ > │ CRYPTO_DRBG_HMAC [=y] && CRYPTO [=y] && CRYPTO_DRBG_MENU [=m] || > CRYPTO_DEV_OMAP_SHAM [=n] && CRYPTO [=y] && CRYPTO_HW [=y] && \ > │ > │ ARCH_OMAP2PLUS [=n] || CRYPTO_DEV_QAT [=n] && CRYPTO [=y] && CRYPTO_HW > [=y] │ > > > What does (IPV6 [=n] || IPV6 [=n]=n) mean? > Is this a bug in my configuration? No, this comes from the definition of IP_SCTP in net/sctp/Kconfig. menuconfig IP_SCTP tristate "The SCTP Protocol" depends on INET depends on IPV6 || IPV6=n ^^ This weird-looking condition has the effect that: - if IPV6 is disabled, IP_SCTP can be anything ("y","m" or "n") - if IPV6 is enabled to "y", IP_SCTP can also be set to anything. - (and this is the reason why it's there:) if IPV6 is a module ("m"), IP_SCTP can only be "m" or "n". In a way, the "depends on" does not only reference the symbol, but also keeps track of the 'state' of the symbol it references, such that if the dependency (here: IPV6) is to be built as a loadable module only, the dependant feature can also only be built as a loadable module. I can't find the Documentation for that at the moment, but I'm sure that I read this at some point, maybe also only on a mailing list... Regards, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: parisc: adjust L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 CPUs
Hi Helge, today's linux-next tree (next-20150903) contains commit 20f924902ff6 ("parisc: adjust L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 CPUs") which you authored. I noticed it because we[0] are running a daily analysis on all commits in linux-next as part of our research and our tools reported it. In the patch, you create the following #if defined() structure in arch/parisc/include/asm/cache.h (lines 16 and following): #if defined(CONFIG_PA8X00) ... #elif defined(CONFIG_PA20) ... #else ... #endif In Kconfig, CONFIG_PA20 is defined as the following (arch/parisc/Kconfig, line 163): config PA20 def_bool y depends on PA8X00 This means that CONFIG_PA20 can and will only be enabled if CONFIG_PA8X00 has already been enabled, which means that the contents of the "#elif defined(CONFIG_PA20)" block can never be reached: its condition is only evaluated if CONFIG_PA8X00 is disabled, but then CONFIG_PA20 can never be enabled either. Best regards, Andreas [0] https://cados.cs.fau.de -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: parisc: adjust L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 CPUs
Hi Helge, today's linux-next tree (next-20150903) contains commit 20f924902ff6 ("parisc: adjust L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 CPUs") which you authored. I noticed it because we[0] are running a daily analysis on all commits in linux-next as part of our research and our tools reported it. In the patch, you create the following #if defined() structure in arch/parisc/include/asm/cache.h (lines 16 and following): #if defined(CONFIG_PA8X00) ... #elif defined(CONFIG_PA20) ... #else ... #endif In Kconfig, CONFIG_PA20 is defined as the following (arch/parisc/Kconfig, line 163): config PA20 def_bool y depends on PA8X00 This means that CONFIG_PA20 can and will only be enabled if CONFIG_PA8X00 has already been enabled, which means that the contents of the "#elif defined(CONFIG_PA20)" block can never be reached: its condition is only evaluated if CONFIG_PA8X00 is disabled, but then CONFIG_PA20 can never be enabled either. Best regards, Andreas [0] https://cados.cs.fau.de -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/