[PATCH v2 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization

[Du Changbin]

This will apply GCC '-Og' optimization level which is supported
since GCC 4.8. This optimization level offers a reasonable level
of optimization while maintaining fast compilation and a good
debugging experience. It is similar to '-O1' while perferring
to keep debug ability over runtime speed.

If enabling this option breaks your kernel, you should either
disable this or find a fix (mostly in the arch code). Currently
this option has only been tested on x86_64 and arm platform.

This option can satisfy people who was searching for a method
to disable compiler optimizations so to achieve better kernel
debugging experience with kgdb or qemu.

The main problem of '-Og' is we must not use __attribute__((error(msg))).
The compiler will report error though the call to error function
still can be optimize out. So we must fallback to array tricky.

Comparison of vmlinux size: a bit smaller.

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
22665554   9709674  2920908 3529613621a9388 vmlinux

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
21499032   10102758 2920908 3452269820ec64a vmlinux

Comparison of system performance: a bit drop (~6%).
This benchmark of kernel compilation is suggested by Ingo Molnar.
https://lkml.org/lkml/2018/5/2/74

Preparation: Set cpufreq to 'performance'.
for ((cpu=0; cpu<120; cpu++)); do
  G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
  [ -f $G ] && echo performance > $G
done

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

219.764246652 seconds time elapsed   ( +-  0.78% )

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

 233.574187771 seconds time elapsed  ( +-  0.19% )

Signed-off-by: Du Changbin 
Acked-by: Steven Rostedt (VMware) 
---
 Makefile |  5 +
 include/linux/compiler-gcc.h |  2 +-
 include/linux/compiler.h |  2 +-
 init/Kconfig | 19 +++
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 757d6507cb5c..ea908cfe8594 100644
--- a/Makefile
+++ b/Makefile
@@ -657,6 +657,10 @@ KBUILD_CFLAGS  += $(call cc-disable-warning, 
format-truncation)
 KBUILD_CFLAGS  += $(call cc-disable-warning, format-overflow)
 KBUILD_CFLAGS  += $(call cc-disable-warning, int-in-bool-context)
 
+ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
+KBUILD_CFLAGS  += $(call cc-option, -Og)
+KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
+else
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
 KBUILD_CFLAGS  += $(call cc-option,-Oz,-Os)
 KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
@@ -667,6 +671,7 @@ else
 KBUILD_CFLAGS   += -O2
 endif
 endif
+endif
 
 KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
$(call cc-disable-warning,maybe-uninitialized,))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 4d36b27214fd..2a76f7c64b54 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -85,7 +85,7 @@
 
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 
-#ifndef __CHECKER__
+#if !defined(__CHECKER__) && !defined(CONFIG_CC_OPTIMIZE_FOR_DEBUGGING)
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866efb1e..9385c62e9f00 10

[PATCH v2 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization

[Du Changbin]

This will apply GCC '-Og' optimization level which is supported
since GCC 4.8. This optimization level offers a reasonable level
of optimization while maintaining fast compilation and a good
debugging experience. It is similar to '-O1' while perferring
to keep debug ability over runtime speed.

If enabling this option breaks your kernel, you should either
disable this or find a fix (mostly in the arch code). Currently
this option has only been tested on x86_64 and arm platform.

This option can satisfy people who was searching for a method
to disable compiler optimizations so to achieve better kernel
debugging experience with kgdb or qemu.

The main problem of '-Og' is we must not use __attribute__((error(msg))).
The compiler will report error though the call to error function
still can be optimize out. So we must fallback to array tricky.

Comparison of vmlinux size: a bit smaller.

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
22665554   9709674  2920908 3529613621a9388 vmlinux

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
21499032   10102758 2920908 3452269820ec64a vmlinux

Comparison of system performance: a bit drop (~6%).
This benchmark of kernel compilation is suggested by Ingo Molnar.
https://lkml.org/lkml/2018/5/2/74

Preparation: Set cpufreq to 'performance'.
for ((cpu=0; cpu<120; cpu++)); do
  G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
  [ -f $G ] && echo performance > $G
done

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

219.764246652 seconds time elapsed   ( +-  0.78% )

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

 233.574187771 seconds time elapsed  ( +-  0.19% )

Signed-off-by: Du Changbin 
Acked-by: Steven Rostedt (VMware) 
---
 Makefile |  5 +
 include/linux/compiler-gcc.h |  2 +-
 include/linux/compiler.h |  2 +-
 init/Kconfig | 19 +++
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 757d6507cb5c..ea908cfe8594 100644
--- a/Makefile
+++ b/Makefile
@@ -657,6 +657,10 @@ KBUILD_CFLAGS  += $(call cc-disable-warning, 
format-truncation)
 KBUILD_CFLAGS  += $(call cc-disable-warning, format-overflow)
 KBUILD_CFLAGS  += $(call cc-disable-warning, int-in-bool-context)
 
+ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
+KBUILD_CFLAGS  += $(call cc-option, -Og)
+KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
+else
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
 KBUILD_CFLAGS  += $(call cc-option,-Oz,-Os)
 KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
@@ -667,6 +671,7 @@ else
 KBUILD_CFLAGS   += -O2
 endif
 endif
+endif
 
 KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
$(call cc-disable-warning,maybe-uninitialized,))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 4d36b27214fd..2a76f7c64b54 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -85,7 +85,7 @@
 
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 
-#ifndef __CHECKER__
+#if !defined(__CHECKER__) && !defined(CONFIG_CC_OPTIMIZE_FOR_DEBUGGING)
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866efb1e..9385c62e9f00 10

[PATCH v2 3/4] ARM: mm: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING

[Du Changbin]

With '-Og' optimization level, GCC would not optimize a count for a loop
as a constant value. But BUILD_BUG_ON() only accept compile-time constant
values. Let's use __fix_to_virt() to avoid the error.

arch/arm/mm/mmu.o: In function `fix_to_virt':
/home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined 
reference to `__compiletime_assert_31'
Makefile:1051: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1

Signed-off-by: Du Changbin 
Acked-by: Steven Rostedt (VMware) 
---
 arch/arm/mm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e46a6a446cdd..c08d74e76714 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
pte_t *pte;
struct map_desc map;
 
-   map.virtual = fix_to_virt(i);
+   map.virtual = __fix_to_virt(i);
pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), 
map.virtual);
 
/* Only i/o device mappings are supported ATM */
-- 
2.17.1

[PATCH v2 3/4] ARM: mm: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING

[Du Changbin]

With '-Og' optimization level, GCC would not optimize a count for a loop
as a constant value. But BUILD_BUG_ON() only accept compile-time constant
values. Let's use __fix_to_virt() to avoid the error.

arch/arm/mm/mmu.o: In function `fix_to_virt':
/home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined 
reference to `__compiletime_assert_31'
Makefile:1051: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1

Signed-off-by: Du Changbin 
Acked-by: Steven Rostedt (VMware) 
---
 arch/arm/mm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e46a6a446cdd..c08d74e76714 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
pte_t *pte;
struct map_desc map;
 
-   map.virtual = fix_to_virt(i);
+   map.virtual = __fix_to_virt(i);
pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), 
map.virtual);
 
/* Only i/o device mappings are supported ATM */
-- 
2.17.1

[PATCH v2 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du Changbin]

This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
this option will prevent the compiler from optimizing the kernel by
auto-inlining functions not marked with the inline keyword.

With this option, only functions explicitly marked with "inline" will
be inlined. This will allow the function tracer to trace more functions
because it only traces functions that the compiler has not inlined.

Signed-off-by: Du Changbin 
Acked-by: Steven Rostedt (VMware) 
---
 Makefile  |  6 ++
 lib/Kconfig.debug | 17 +
 2 files changed, 23 insertions(+)

diff --git a/Makefile b/Makefile
index e8b599b4dcde..757d6507cb5c 100644
--- a/Makefile
+++ b/Makefile
@@ -749,6 +749,12 @@ KBUILD_CFLAGS  += $(call cc-option, 
-femit-struct-debug-baseonly) \
   $(call cc-option,-fno-var-tracking)
 endif
 
+ifdef CONFIG_NO_AUTO_INLINE
+KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
+  $(call cc-option, -fno-inline-small-functions) \
+  $(call cc-option, -fno-inline-functions-called-once)
+endif
+
 ifdef CONFIG_FUNCTION_TRACER
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
   # gcc 5 supports generating the mcount tables directly
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4966c4fbe7f7..c7c28ee01dfc 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -211,6 +211,23 @@ config GDB_SCRIPTS
  instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
  for further details.
 
+config NO_AUTO_INLINE
+   bool "Disable compiler auto-inline optimizations"
+   help
+ This will prevent the compiler from optimizing the kernel by
+ auto-inlining functions not marked with the inline keyword.
+ With this option, only functions explicitly marked with
+ "inline" will be inlined. This will allow the function tracer
+ to trace more functions because it only traces functions that
+ the compiler has not inlined.
+
+ Enabling this function can help debugging a kernel if using
+ the function tracer. But it can also change how the kernel
+ works, because inlining functions may change the timing,
+ which could make it difficult while debugging race conditions.
+
+ If unsure, select N.
+
 config ENABLE_MUST_CHECK
bool "Enable __must_check logic"
default y
-- 
2.17.1

[PATCH v2 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du Changbin]

This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
this option will prevent the compiler from optimizing the kernel by
auto-inlining functions not marked with the inline keyword.

With this option, only functions explicitly marked with "inline" will
be inlined. This will allow the function tracer to trace more functions
because it only traces functions that the compiler has not inlined.

Signed-off-by: Du Changbin 
Acked-by: Steven Rostedt (VMware) 
---
 Makefile  |  6 ++
 lib/Kconfig.debug | 17 +
 2 files changed, 23 insertions(+)

diff --git a/Makefile b/Makefile
index e8b599b4dcde..757d6507cb5c 100644
--- a/Makefile
+++ b/Makefile
@@ -749,6 +749,12 @@ KBUILD_CFLAGS  += $(call cc-option, 
-femit-struct-debug-baseonly) \
   $(call cc-option,-fno-var-tracking)
 endif
 
+ifdef CONFIG_NO_AUTO_INLINE
+KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
+  $(call cc-option, -fno-inline-small-functions) \
+  $(call cc-option, -fno-inline-functions-called-once)
+endif
+
 ifdef CONFIG_FUNCTION_TRACER
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
   # gcc 5 supports generating the mcount tables directly
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4966c4fbe7f7..c7c28ee01dfc 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -211,6 +211,23 @@ config GDB_SCRIPTS
  instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
  for further details.
 
+config NO_AUTO_INLINE
+   bool "Disable compiler auto-inline optimizations"
+   help
+ This will prevent the compiler from optimizing the kernel by
+ auto-inlining functions not marked with the inline keyword.
+ With this option, only functions explicitly marked with
+ "inline" will be inlined. This will allow the function tracer
+ to trace more functions because it only traces functions that
+ the compiler has not inlined.
+
+ Enabling this function can help debugging a kernel if using
+ the function tracer. But it can also change how the kernel
+ works, because inlining functions may change the timing,
+ which could make it difficult while debugging race conditions.
+
+ If unsure, select N.
+
 config ENABLE_MUST_CHECK
bool "Enable __must_check logic"
default y
-- 
2.17.1

[PATCH v2 0/4] kernel hacking: GCC optimization for better debug experience (-Og)

[Du Changbin]

Hi all,
I have posted this series several months ago but interrupted by personal
affairs. Now I get time to complete this task. Thanks for all of the
reviewers.

I know some kernel developers was searching for a method to dissable GCC
optimizations, probably they want to apply GCC '-O0' option. But since Linux
kernel relies on GCC optimization to remove some dead code, so '-O0' just
breaks the build. They do need this because they want to debug kernel with
qemu, simics, kgtp or kgdb.

Thanks for the GCC '-Og' optimization level introduced in GCC 4.8, which
offers a reasonable level of optimization while maintaining fast compilation
and a good debugging experience. It is similar to '-O1' while perferring to
keep debug ability over runtime speed. With '-Og', we can build a kernel with
better debug ability and little performance drop after some simple change.

In this series, firstly introduce a new config CONFIG_NO_AUTO_INLINE after two
fixes for this new option. With this option, only functions explicitly marked
with "inline" will  be inlined. This will allow the function tracer to trace
more functions because it only traces functions that the compiler has not
inlined.

Then introduce new config CC_OPTIMIZE_FOR_DEBUGGING which apply '-Og'
optimization level for whole kernel, with a simple fix in fix_to_virt().
Currently I have only tested this option on x86 and ARM platform. Other
platforms should also work but probably need some compiling fixes as what
having done in this series. I leave that to who want to try this debug
option.

Comparison of vmlinux size: a bit smaller.

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
22665554   9709674  2920908 3529613621a9388 vmlinux

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
21499032   10102758 2920908 3452269820ec64a vmlinux


Comparison of system performance: a bit drop (~6%).
This benchmark of kernel compilation is suggested by Ingo Molnar.
https://lkml.org/lkml/2018/5/2/74

Preparation: Set cpufreq to 'performance'.
for ((cpu=0; cpu<120; cpu++)); do
  G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
  [ -f $G ] && echo performance > $G
done

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

 Performance counter stats for 'make -j8' (5 runs):

219.764246652 seconds time elapsed   ( +-  0.78% )

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

 233.574187771 seconds time elapsed  ( +-  0.19% )


Du Changbin (4):
  x86/mm: surround level4_kernel_pgt with #ifdef
CONFIG_X86_5LEVEL...#endif
  kernel hacking: new config NO_AUTO_INLINE to disable compiler
auto-inline optimizations
  ARM: mm: fix build error in fix_to_virt with
CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
  kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og
optimization

 Makefile  | 11 +++
 arch/arm/mm/mmu.c |  2 +-
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c  | 13 ++---
 include/linux/compiler-gcc.h  |  2 +-
 include/linux/compiler.h  |  2 +-
 init/Kconfig  | 19 +++
 lib/Kconfig.debug | 17 +
 8 files changed, 58 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif

[Du Changbin]

The level4_kernel_pgt is only defined when X86_5LEVEL is enabled. So
surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif to
make code correct.

Signed-off-by: Du Changbin 
Acked-by: Steven Rostedt (VMware) 
---
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c  | 13 ++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_64.h 
b/arch/x86/include/asm/pgtable_64.h
index 9c85b54bf03c..9333f7fa5bdb 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -16,7 +16,9 @@
 #include 
 #include 
 
+#ifdef CONFIG_X86_5LEVEL
 extern p4d_t level4_kernel_pgt[512];
+#endif
 extern p4d_t level4_ident_pgt[512];
 extern pud_t level3_kernel_pgt[512];
 extern pud_t level3_ident_pgt[512];
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index ddee1f0870c4..4a59ef93c258 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -151,16 +151,15 @@ unsigned long __head __startup_64(unsigned long physaddr,
 
pgd = fixup_pointer(_top_pgt, physaddr);
p = pgd + pgd_index(__START_KERNEL_map);
-   if (la57)
-   *p = (unsigned long)level4_kernel_pgt;
-   else
-   *p = (unsigned long)level3_kernel_pgt;
-   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
-
+#ifdef CONFIG_X86_5LEVEL
if (la57) {
+   *p = (unsigned long)level4_kernel_pgt;
p4d = fixup_pointer(_kernel_pgt, physaddr);
p4d[511] += load_delta;
-   }
+   } else
+#endif
+   *p = (unsigned long)level3_kernel_pgt;
+   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
 
pud = fixup_pointer(_kernel_pgt, physaddr);
pud[510] += load_delta;
-- 
2.17.1

[PATCH v2 0/4] kernel hacking: GCC optimization for better debug experience (-Og)

[Du Changbin]

Hi all,
I have posted this series several months ago but interrupted by personal
affairs. Now I get time to complete this task. Thanks for all of the
reviewers.

I know some kernel developers was searching for a method to dissable GCC
optimizations, probably they want to apply GCC '-O0' option. But since Linux
kernel relies on GCC optimization to remove some dead code, so '-O0' just
breaks the build. They do need this because they want to debug kernel with
qemu, simics, kgtp or kgdb.

Thanks for the GCC '-Og' optimization level introduced in GCC 4.8, which
offers a reasonable level of optimization while maintaining fast compilation
and a good debugging experience. It is similar to '-O1' while perferring to
keep debug ability over runtime speed. With '-Og', we can build a kernel with
better debug ability and little performance drop after some simple change.

In this series, firstly introduce a new config CONFIG_NO_AUTO_INLINE after two
fixes for this new option. With this option, only functions explicitly marked
with "inline" will  be inlined. This will allow the function tracer to trace
more functions because it only traces functions that the compiler has not
inlined.

Then introduce new config CC_OPTIMIZE_FOR_DEBUGGING which apply '-Og'
optimization level for whole kernel, with a simple fix in fix_to_virt().
Currently I have only tested this option on x86 and ARM platform. Other
platforms should also work but probably need some compiling fixes as what
having done in this series. I leave that to who want to try this debug
option.

Comparison of vmlinux size: a bit smaller.

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
22665554   9709674  2920908 3529613621a9388 vmlinux

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
21499032   10102758 2920908 3452269820ec64a vmlinux


Comparison of system performance: a bit drop (~6%).
This benchmark of kernel compilation is suggested by Ingo Molnar.
https://lkml.org/lkml/2018/5/2/74

Preparation: Set cpufreq to 'performance'.
for ((cpu=0; cpu<120; cpu++)); do
  G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
  [ -f $G ] && echo performance > $G
done

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

 Performance counter stats for 'make -j8' (5 runs):

219.764246652 seconds time elapsed   ( +-  0.78% )

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

 233.574187771 seconds time elapsed  ( +-  0.19% )


Du Changbin (4):
  x86/mm: surround level4_kernel_pgt with #ifdef
CONFIG_X86_5LEVEL...#endif
  kernel hacking: new config NO_AUTO_INLINE to disable compiler
auto-inline optimizations
  ARM: mm: fix build error in fix_to_virt with
CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
  kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og
optimization

 Makefile  | 11 +++
 arch/arm/mm/mmu.c |  2 +-
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c  | 13 ++---
 include/linux/compiler-gcc.h  |  2 +-
 include/linux/compiler.h  |  2 +-
 init/Kconfig  | 19 +++
 lib/Kconfig.debug | 17 +
 8 files changed, 58 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif

[Du Changbin]

The level4_kernel_pgt is only defined when X86_5LEVEL is enabled. So
surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif to
make code correct.

Signed-off-by: Du Changbin 
Acked-by: Steven Rostedt (VMware) 
---
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c  | 13 ++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_64.h 
b/arch/x86/include/asm/pgtable_64.h
index 9c85b54bf03c..9333f7fa5bdb 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -16,7 +16,9 @@
 #include 
 #include 
 
+#ifdef CONFIG_X86_5LEVEL
 extern p4d_t level4_kernel_pgt[512];
+#endif
 extern p4d_t level4_ident_pgt[512];
 extern pud_t level3_kernel_pgt[512];
 extern pud_t level3_ident_pgt[512];
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index ddee1f0870c4..4a59ef93c258 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -151,16 +151,15 @@ unsigned long __head __startup_64(unsigned long physaddr,
 
pgd = fixup_pointer(_top_pgt, physaddr);
p = pgd + pgd_index(__START_KERNEL_map);
-   if (la57)
-   *p = (unsigned long)level4_kernel_pgt;
-   else
-   *p = (unsigned long)level3_kernel_pgt;
-   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
-
+#ifdef CONFIG_X86_5LEVEL
if (la57) {
+   *p = (unsigned long)level4_kernel_pgt;
p4d = fixup_pointer(_kernel_pgt, physaddr);
p4d[511] += load_delta;
-   }
+   } else
+#endif
+   *p = (unsigned long)level3_kernel_pgt;
+   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
 
pud = fixup_pointer(_kernel_pgt, physaddr);
pud[510] += load_delta;
-- 
2.17.1

Re: [PATCH 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du Changbin]

On Thu, Oct 18, 2018 at 12:59:48PM -0400, Steven Rostedt wrote:
> On Thu, 18 Oct 2018 16:25:46 +
> Du Changbin  wrote:
> 
> > From: Changbin Du 
> > 
> > This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
> > this option will prevent the compiler from optimizing the kernel by
> > auto-inlining functions not marked with the inline keyword.
> > 
> > With this option, only functions explicitly marked with "inline" will
> > be inlined. This will allow the function tracer to trace more functions
> > because it only traces functions that the compiler has not inlined.
> > 
> > Signed-off-by: Changbin Du 
> > Acked-by: Steven Rostedt (VMware) 
> 
> I have acked patch this before, but this particular patch has extra
> changes that I have not acked.
>
Steven, no extra changes made. I just wronly rebased it on top of mainline.
:)

> 
> 
> > +config ENABLE_WARN_DEPRECATED
> > +   bool "Enable __deprecated logic"
> > +   default y
> > +   help
> > + Enable the __deprecated logic in the kernel build.
> > + Disable this to suppress the "warning: 'foo' is deprecated
> > + (declared at kernel/power/somefile.c:1234)" messages.
> > +
> 
> What is that?
> 
> -- Steve

-- 
Thanks,
Du Changbin

Re: [PATCH 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du Changbin]

On Thu, Oct 18, 2018 at 12:59:48PM -0400, Steven Rostedt wrote:
> On Thu, 18 Oct 2018 16:25:46 +
> Du Changbin  wrote:
> 
> > From: Changbin Du 
> > 
> > This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
> > this option will prevent the compiler from optimizing the kernel by
> > auto-inlining functions not marked with the inline keyword.
> > 
> > With this option, only functions explicitly marked with "inline" will
> > be inlined. This will allow the function tracer to trace more functions
> > because it only traces functions that the compiler has not inlined.
> > 
> > Signed-off-by: Changbin Du 
> > Acked-by: Steven Rostedt (VMware) 
> 
> I have acked patch this before, but this particular patch has extra
> changes that I have not acked.
>
Steven, no extra changes made. I just wronly rebased it on top of mainline.
:)

> 
> 
> > +config ENABLE_WARN_DEPRECATED
> > +   bool "Enable __deprecated logic"
> > +   default y
> > +   help
> > + Enable the __deprecated logic in the kernel build.
> > + Disable this to suppress the "warning: 'foo' is deprecated
> > + (declared at kernel/power/somefile.c:1234)" messages.
> > +
> 
> What is that?
> 
> -- Steve

-- 
Thanks,
Du Changbin

Re: [PATCH 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du Changbin]

On Thu, Oct 18, 2018 at 05:57:56PM +0100, Robin Murphy wrote:
> On 18/10/18 17:25, Du Changbin wrote:
> > From: Changbin Du 
> > 
> > This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
> > this option will prevent the compiler from optimizing the kernel by
> > auto-inlining functions not marked with the inline keyword.
> > 
> > With this option, only functions explicitly marked with "inline" will
> > be inlined. This will allow the function tracer to trace more functions
> > because it only traces functions that the compiler has not inlined.
> > 
> > Signed-off-by: Changbin Du 
> > Acked-by: Steven Rostedt (VMware) 
> > ---
> >   Makefile  |  6 ++
> >   lib/Kconfig.debug | 25 +
> >   2 files changed, 31 insertions(+)
> > 
> > diff --git a/Makefile b/Makefile
> > index e8b599b4dcde..757d6507cb5c 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -749,6 +749,12 @@ KBUILD_CFLAGS  += $(call cc-option, 
> > -femit-struct-debug-baseonly) \
> >$(call cc-option,-fno-var-tracking)
> >   endif
> > +ifdef CONFIG_NO_AUTO_INLINE
> > +KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
> > +  $(call cc-option, -fno-inline-small-functions) \
> > +  $(call cc-option, -fno-inline-functions-called-once)
> > +endif
> > +
> >   ifdef CONFIG_FUNCTION_TRACER
> >   ifdef CONFIG_FTRACE_MCOUNT_RECORD
> > # gcc 5 supports generating the mcount tables directly
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 4966c4fbe7f7..0f9b4fa78b1c 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -211,6 +211,31 @@ config GDB_SCRIPTS
> >   instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
> >   for further details.
> > +config NO_AUTO_INLINE
> > +   bool "Disable compiler auto-inline optimizations"
> > +   help
> > + This will prevent the compiler from optimizing the kernel by
> > + auto-inlining functions not marked with the inline keyword.
> > + With this option, only functions explicitly marked with
> > + "inline" will be inlined. This will allow the function tracer
> > + to trace more functions because it only traces functions that
> > + the compiler has not inlined.
> > +
> > + Enabling this function can help debugging a kernel if using
> > + the function tracer. But it can also change how the kernel
> > + works, because inlining functions may change the timing,
> > + which could make it difficult while debugging race conditions.
> > +
> > + If unsure, select N.
> > +
> > +config ENABLE_WARN_DEPRECATED
> 
> This part doesn't look like it belongs in this patch, and judging by the
> commit message in 771c035372a0 wouldn't be welcome back anyway.
> 
opps, this is a rebasing mistake. Let me update it. Thanks.

> Robin.
> 
> > +   bool "Enable __deprecated logic"
> > +   default y
> > +   help
> > + Enable the __deprecated logic in the kernel build.
> > + Disable this to suppress the "warning: 'foo' is deprecated
> > + (declared at kernel/power/somefile.c:1234)" messages.
> > +
> >   config ENABLE_MUST_CHECK
> > bool "Enable __must_check logic"
> > default y
> > 

-- 
Thanks,
Du Changbin

Re: [PATCH 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du Changbin]

On Thu, Oct 18, 2018 at 05:57:56PM +0100, Robin Murphy wrote:
> On 18/10/18 17:25, Du Changbin wrote:
> > From: Changbin Du 
> > 
> > This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
> > this option will prevent the compiler from optimizing the kernel by
> > auto-inlining functions not marked with the inline keyword.
> > 
> > With this option, only functions explicitly marked with "inline" will
> > be inlined. This will allow the function tracer to trace more functions
> > because it only traces functions that the compiler has not inlined.
> > 
> > Signed-off-by: Changbin Du 
> > Acked-by: Steven Rostedt (VMware) 
> > ---
> >   Makefile  |  6 ++
> >   lib/Kconfig.debug | 25 +
> >   2 files changed, 31 insertions(+)
> > 
> > diff --git a/Makefile b/Makefile
> > index e8b599b4dcde..757d6507cb5c 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -749,6 +749,12 @@ KBUILD_CFLAGS  += $(call cc-option, 
> > -femit-struct-debug-baseonly) \
> >$(call cc-option,-fno-var-tracking)
> >   endif
> > +ifdef CONFIG_NO_AUTO_INLINE
> > +KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
> > +  $(call cc-option, -fno-inline-small-functions) \
> > +  $(call cc-option, -fno-inline-functions-called-once)
> > +endif
> > +
> >   ifdef CONFIG_FUNCTION_TRACER
> >   ifdef CONFIG_FTRACE_MCOUNT_RECORD
> > # gcc 5 supports generating the mcount tables directly
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 4966c4fbe7f7..0f9b4fa78b1c 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -211,6 +211,31 @@ config GDB_SCRIPTS
> >   instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
> >   for further details.
> > +config NO_AUTO_INLINE
> > +   bool "Disable compiler auto-inline optimizations"
> > +   help
> > + This will prevent the compiler from optimizing the kernel by
> > + auto-inlining functions not marked with the inline keyword.
> > + With this option, only functions explicitly marked with
> > + "inline" will be inlined. This will allow the function tracer
> > + to trace more functions because it only traces functions that
> > + the compiler has not inlined.
> > +
> > + Enabling this function can help debugging a kernel if using
> > + the function tracer. But it can also change how the kernel
> > + works, because inlining functions may change the timing,
> > + which could make it difficult while debugging race conditions.
> > +
> > + If unsure, select N.
> > +
> > +config ENABLE_WARN_DEPRECATED
> 
> This part doesn't look like it belongs in this patch, and judging by the
> commit message in 771c035372a0 wouldn't be welcome back anyway.
> 
opps, this is a rebasing mistake. Let me update it. Thanks.

> Robin.
> 
> > +   bool "Enable __deprecated logic"
> > +   default y
> > +   help
> > + Enable the __deprecated logic in the kernel build.
> > + Disable this to suppress the "warning: 'foo' is deprecated
> > + (declared at kernel/power/somefile.c:1234)" messages.
> > +
> >   config ENABLE_MUST_CHECK
> > bool "Enable __must_check logic"
> > default y
> > 

-- 
Thanks,
Du Changbin

[PATCH 3/4] ARM: mm: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING

[Du Changbin]

From: Changbin Du 

With '-Og' optimization level, GCC would not optimize a count for a loop
as a constant value. But BUILD_BUG_ON() only accept compile-time constant
values. Let's use __fix_to_virt() to avoid the error.

arch/arm/mm/mmu.o: In function `fix_to_virt':
/home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined 
reference to `__compiletime_assert_31'
Makefile:1051: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1

Signed-off-by: Changbin Du 
Acked-by: Steven Rostedt (VMware) 
---
 arch/arm/mm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e46a6a446cdd..c08d74e76714 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
pte_t *pte;
struct map_desc map;
 
-   map.virtual = fix_to_virt(i);
+   map.virtual = __fix_to_virt(i);
pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), 
map.virtual);
 
/* Only i/o device mappings are supported ATM */
-- 
2.17.1

[PATCH 3/4] ARM: mm: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING

[Du Changbin]

From: Changbin Du 

With '-Og' optimization level, GCC would not optimize a count for a loop
as a constant value. But BUILD_BUG_ON() only accept compile-time constant
values. Let's use __fix_to_virt() to avoid the error.

arch/arm/mm/mmu.o: In function `fix_to_virt':
/home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined 
reference to `__compiletime_assert_31'
Makefile:1051: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1

Signed-off-by: Changbin Du 
Acked-by: Steven Rostedt (VMware) 
---
 arch/arm/mm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e46a6a446cdd..c08d74e76714 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
pte_t *pte;
struct map_desc map;
 
-   map.virtual = fix_to_virt(i);
+   map.virtual = __fix_to_virt(i);
pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), 
map.virtual);
 
/* Only i/o device mappings are supported ATM */
-- 
2.17.1

[PATCH 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization

[Du Changbin]

From: Changbin Du 

This will apply GCC '-Og' optimization level which is supported
since GCC 4.8. This optimization level offers a reasonable level
of optimization while maintaining fast compilation and a good
debugging experience. It is similar to '-O1' while perferring
to keep debug ability over runtime speed.

If enabling this option breaks your kernel, you should either
disable this or find a fix (mostly in the arch code). Currently
this option has only been tested on x86_64 and arm platform.

This option can satisfy people who was searching for a method
to disable compiler optimizations so to achieve better kernel
debugging experience with kgdb or qemu.

The main problem of '-Og' is we must not use __attribute__((error(msg))).
The compiler will report error though the call to error function
still can be optimize out. So we must fallback to array tricky.

Comparison of vmlinux size: a bit smaller.

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
22665554   9709674  2920908 3529613621a9388 vmlinux

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
21499032   10102758 2920908 3452269820ec64a vmlinux

Comparison of system performance: a bit drop (~6%).
This benchmark of kernel compilation is suggested by Ingo Molnar.
https://lkml.org/lkml/2018/5/2/74

Preparation: Set cpufreq to 'performance'.
for ((cpu=0; cpu<120; cpu++)); do
  G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
  [ -f $G ] && echo performance > $G
done

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

219.764246652 seconds time elapsed   ( +-  0.78% )

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

 233.574187771 seconds time elapsed  ( +-  0.19% )

Signed-off-by: Changbin Du 
Acked-by: Steven Rostedt (VMware) 
---
 Makefile |  5 +
 include/linux/compiler-gcc.h |  2 +-
 include/linux/compiler.h |  2 +-
 init/Kconfig | 19 +++
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 757d6507cb5c..0e747fafb53b 100644
--- a/Makefile
+++ b/Makefile
@@ -657,6 +657,10 @@ KBUILD_CFLAGS  += $(call cc-disable-warning, 
format-truncation)
 KBUILD_CFLAGS  += $(call cc-disable-warning, format-overflow)
 KBUILD_CFLAGS  += $(call cc-disable-warning, int-in-bool-context)
 
+ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
+KBUILD_CFLAGS  += $(call cc-option, -Og)
+KBUILD_CFLAGS   += $(call cc-disable-warning,maybe-uninitialized,)
+else
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
 KBUILD_CFLAGS  += $(call cc-option,-Oz,-Os)
 KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
@@ -667,6 +671,7 @@ else
 KBUILD_CFLAGS   += -O2
 endif
 endif
+endif
 
 KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
$(call cc-disable-warning,maybe-uninitialized,))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 4d36b27214fd..2a76f7c64b54 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -85,7 +85,7 @@
 
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 
-#ifndef __CHECKER__
+#if !defined(__CHECKER__) && !defined(CONFIG_CC_OPTIMIZE_FOR_DEBUGGING)
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866efb1e..9385c62e9f00 100644
--- a/include/linux/compiler.h
+++ 

[PATCH 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization

[Du Changbin]

From: Changbin Du 

This will apply GCC '-Og' optimization level which is supported
since GCC 4.8. This optimization level offers a reasonable level
of optimization while maintaining fast compilation and a good
debugging experience. It is similar to '-O1' while perferring
to keep debug ability over runtime speed.

If enabling this option breaks your kernel, you should either
disable this or find a fix (mostly in the arch code). Currently
this option has only been tested on x86_64 and arm platform.

This option can satisfy people who was searching for a method
to disable compiler optimizations so to achieve better kernel
debugging experience with kgdb or qemu.

The main problem of '-Og' is we must not use __attribute__((error(msg))).
The compiler will report error though the call to error function
still can be optimize out. So we must fallback to array tricky.

Comparison of vmlinux size: a bit smaller.

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
22665554   9709674  2920908 3529613621a9388 vmlinux

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
21499032   10102758 2920908 3452269820ec64a vmlinux

Comparison of system performance: a bit drop (~6%).
This benchmark of kernel compilation is suggested by Ingo Molnar.
https://lkml.org/lkml/2018/5/2/74

Preparation: Set cpufreq to 'performance'.
for ((cpu=0; cpu<120; cpu++)); do
  G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
  [ -f $G ] && echo performance > $G
done

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

219.764246652 seconds time elapsed   ( +-  0.78% )

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

 233.574187771 seconds time elapsed  ( +-  0.19% )

Signed-off-by: Changbin Du 
Acked-by: Steven Rostedt (VMware) 
---
 Makefile |  5 +
 include/linux/compiler-gcc.h |  2 +-
 include/linux/compiler.h |  2 +-
 init/Kconfig | 19 +++
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 757d6507cb5c..0e747fafb53b 100644
--- a/Makefile
+++ b/Makefile
@@ -657,6 +657,10 @@ KBUILD_CFLAGS  += $(call cc-disable-warning, 
format-truncation)
 KBUILD_CFLAGS  += $(call cc-disable-warning, format-overflow)
 KBUILD_CFLAGS  += $(call cc-disable-warning, int-in-bool-context)
 
+ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
+KBUILD_CFLAGS  += $(call cc-option, -Og)
+KBUILD_CFLAGS   += $(call cc-disable-warning,maybe-uninitialized,)
+else
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
 KBUILD_CFLAGS  += $(call cc-option,-Oz,-Os)
 KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
@@ -667,6 +671,7 @@ else
 KBUILD_CFLAGS   += -O2
 endif
 endif
+endif
 
 KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
$(call cc-disable-warning,maybe-uninitialized,))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 4d36b27214fd..2a76f7c64b54 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -85,7 +85,7 @@
 
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 
-#ifndef __CHECKER__
+#if !defined(__CHECKER__) && !defined(CONFIG_CC_OPTIMIZE_FOR_DEBUGGING)
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866efb1e..9385c62e9f00 100644
--- a/include/linux/compiler.h
+++ 

[PATCH 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif

[Du Changbin]

From: Changbin Du 

The level4_kernel_pgt is only defined when X86_5LEVEL is enabled. So
surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif to
make code correct.

Signed-off-by: Changbin Du 
Acked-by: Steven Rostedt (VMware) 
---
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c  | 13 ++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_64.h 
b/arch/x86/include/asm/pgtable_64.h
index 9c85b54bf03c..9333f7fa5bdb 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -16,7 +16,9 @@
 #include 
 #include 
 
+#ifdef CONFIG_X86_5LEVEL
 extern p4d_t level4_kernel_pgt[512];
+#endif
 extern p4d_t level4_ident_pgt[512];
 extern pud_t level3_kernel_pgt[512];
 extern pud_t level3_ident_pgt[512];
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index ddee1f0870c4..4a59ef93c258 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -151,16 +151,15 @@ unsigned long __head __startup_64(unsigned long physaddr,
 
pgd = fixup_pointer(_top_pgt, physaddr);
p = pgd + pgd_index(__START_KERNEL_map);
-   if (la57)
-   *p = (unsigned long)level4_kernel_pgt;
-   else
-   *p = (unsigned long)level3_kernel_pgt;
-   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
-
+#ifdef CONFIG_X86_5LEVEL
if (la57) {
+   *p = (unsigned long)level4_kernel_pgt;
p4d = fixup_pointer(_kernel_pgt, physaddr);
p4d[511] += load_delta;
-   }
+   } else
+#endif
+   *p = (unsigned long)level3_kernel_pgt;
+   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
 
pud = fixup_pointer(_kernel_pgt, physaddr);
pud[510] += load_delta;
-- 
2.17.1

[PATCH 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du Changbin]

From: Changbin Du 

This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
this option will prevent the compiler from optimizing the kernel by
auto-inlining functions not marked with the inline keyword.

With this option, only functions explicitly marked with "inline" will
be inlined. This will allow the function tracer to trace more functions
because it only traces functions that the compiler has not inlined.

Signed-off-by: Changbin Du 
Acked-by: Steven Rostedt (VMware) 
---
 Makefile  |  6 ++
 lib/Kconfig.debug | 25 +
 2 files changed, 31 insertions(+)

diff --git a/Makefile b/Makefile
index e8b599b4dcde..757d6507cb5c 100644
--- a/Makefile
+++ b/Makefile
@@ -749,6 +749,12 @@ KBUILD_CFLAGS  += $(call cc-option, 
-femit-struct-debug-baseonly) \
   $(call cc-option,-fno-var-tracking)
 endif
 
+ifdef CONFIG_NO_AUTO_INLINE
+KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
+  $(call cc-option, -fno-inline-small-functions) \
+  $(call cc-option, -fno-inline-functions-called-once)
+endif
+
 ifdef CONFIG_FUNCTION_TRACER
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
   # gcc 5 supports generating the mcount tables directly
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4966c4fbe7f7..0f9b4fa78b1c 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -211,6 +211,31 @@ config GDB_SCRIPTS
  instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
  for further details.
 
+config NO_AUTO_INLINE
+   bool "Disable compiler auto-inline optimizations"
+   help
+ This will prevent the compiler from optimizing the kernel by
+ auto-inlining functions not marked with the inline keyword.
+ With this option, only functions explicitly marked with
+ "inline" will be inlined. This will allow the function tracer
+ to trace more functions because it only traces functions that
+ the compiler has not inlined.
+
+ Enabling this function can help debugging a kernel if using
+ the function tracer. But it can also change how the kernel
+ works, because inlining functions may change the timing,
+ which could make it difficult while debugging race conditions.
+
+ If unsure, select N.
+
+config ENABLE_WARN_DEPRECATED
+   bool "Enable __deprecated logic"
+   default y
+   help
+ Enable the __deprecated logic in the kernel build.
+ Disable this to suppress the "warning: 'foo' is deprecated
+ (declared at kernel/power/somefile.c:1234)" messages.
+
 config ENABLE_MUST_CHECK
bool "Enable __must_check logic"
default y
-- 
2.17.1

[PATCH 0/4] kernel hacking: GCC optimization for better debug experience (-Og)

[Du Changbin]

Hi all,
I have posted this series several months ago but interrupted by personal
affairs. Now I get time to complete this task. Thanks for all of the
reviewers.

I know some kernel developers was searching for a method to dissable GCC
optimizations, probably they want to apply GCC '-O0' option. But since Linux
kernel replys on GCC optimization to remove some dead code, so '-O0' just
breaks the build. They do need this because they want to debug kernel with
qemu, simics, kgtp or kgdb.

Thanks for the GCC '-Og' optimization level introduced in GCC 4.8, which
offers a reasonable level of optimization while maintaining fast compilation
and a good debugging experience. It is similar to '-O1' while perferring to
keep debug ability over runtime speed. With '-Og', we can build a kernel with
better debug ability and little performance drop after some simple change.

In this series, firstly introduce a new config CONFIG_NO_AUTO_INLINE after two
fixes for this new option. With this option, only functions explicitly marked
with "inline" will  be inlined. This will allow the function tracer to trace
more functions because it only traces functions that the compiler has not
inlined.

Then introduce new config CC_OPTIMIZE_FOR_DEBUGGING which apply '-Og'
optimization level for whole kernel, with a simple fix in fix_to_virt().
Currently I have only tested this option on x86 and ARM platform. Other
platforms should also work but probably need some compiling fixes as what
having done in this series. I leave that to who want to try this debug
option.

Comparison of vmlinux size: a bit smaller.

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
22665554   9709674  2920908 3529613621a9388 vmlinux

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
21499032   10102758 2920908 3452269820ec64a vmlinux


Comparison of system performance: a bit drop (~6%).
This benchmark of kernel compilation is suggested by Ingo Molnar.
https://lkml.org/lkml/2018/5/2/74

Preparation: Set cpufreq to 'performance'.
for ((cpu=0; cpu<120; cpu++)); do
  G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
  [ -f $G ] && echo performance > $G
done

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

 Performance counter stats for 'make -j8' (5 runs):

219.764246652 seconds time elapsed   ( +-  0.78% )

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

 233.574187771 seconds time elapsed  ( +-  0.19% )


Changbin Du (4):
  x86/mm: surround level4_kernel_pgt with #ifdef
CONFIG_X86_5LEVEL...#endif
  kernel hacking: new config NO_AUTO_INLINE to disable compiler
auto-inline optimizations
  ARM: mm: fix build error in fix_to_virt with
CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
  kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og
optimization

 Makefile  | 11 +++
 arch/arm/mm/mmu.c |  2 +-
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c  | 13 ++---
 include/linux/compiler-gcc.h  |  2 +-
 include/linux/compiler.h  |  2 +-
 init/Kconfig  | 19 +++
 lib/Kconfig.debug | 25 +
 8 files changed, 66 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif

[Du Changbin]

From: Changbin Du 

The level4_kernel_pgt is only defined when X86_5LEVEL is enabled. So
surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif to
make code correct.

Signed-off-by: Changbin Du 
Acked-by: Steven Rostedt (VMware) 
---
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c  | 13 ++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_64.h 
b/arch/x86/include/asm/pgtable_64.h
index 9c85b54bf03c..9333f7fa5bdb 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -16,7 +16,9 @@
 #include 
 #include 
 
+#ifdef CONFIG_X86_5LEVEL
 extern p4d_t level4_kernel_pgt[512];
+#endif
 extern p4d_t level4_ident_pgt[512];
 extern pud_t level3_kernel_pgt[512];
 extern pud_t level3_ident_pgt[512];
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index ddee1f0870c4..4a59ef93c258 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -151,16 +151,15 @@ unsigned long __head __startup_64(unsigned long physaddr,
 
pgd = fixup_pointer(_top_pgt, physaddr);
p = pgd + pgd_index(__START_KERNEL_map);
-   if (la57)
-   *p = (unsigned long)level4_kernel_pgt;
-   else
-   *p = (unsigned long)level3_kernel_pgt;
-   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
-
+#ifdef CONFIG_X86_5LEVEL
if (la57) {
+   *p = (unsigned long)level4_kernel_pgt;
p4d = fixup_pointer(_kernel_pgt, physaddr);
p4d[511] += load_delta;
-   }
+   } else
+#endif
+   *p = (unsigned long)level3_kernel_pgt;
+   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
 
pud = fixup_pointer(_kernel_pgt, physaddr);
pud[510] += load_delta;
-- 
2.17.1

[PATCH 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du Changbin]

From: Changbin Du 

This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
this option will prevent the compiler from optimizing the kernel by
auto-inlining functions not marked with the inline keyword.

With this option, only functions explicitly marked with "inline" will
be inlined. This will allow the function tracer to trace more functions
because it only traces functions that the compiler has not inlined.

Signed-off-by: Changbin Du 
Acked-by: Steven Rostedt (VMware) 
---
 Makefile  |  6 ++
 lib/Kconfig.debug | 25 +
 2 files changed, 31 insertions(+)

diff --git a/Makefile b/Makefile
index e8b599b4dcde..757d6507cb5c 100644
--- a/Makefile
+++ b/Makefile
@@ -749,6 +749,12 @@ KBUILD_CFLAGS  += $(call cc-option, 
-femit-struct-debug-baseonly) \
   $(call cc-option,-fno-var-tracking)
 endif
 
+ifdef CONFIG_NO_AUTO_INLINE
+KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
+  $(call cc-option, -fno-inline-small-functions) \
+  $(call cc-option, -fno-inline-functions-called-once)
+endif
+
 ifdef CONFIG_FUNCTION_TRACER
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
   # gcc 5 supports generating the mcount tables directly
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4966c4fbe7f7..0f9b4fa78b1c 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -211,6 +211,31 @@ config GDB_SCRIPTS
  instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
  for further details.
 
+config NO_AUTO_INLINE
+   bool "Disable compiler auto-inline optimizations"
+   help
+ This will prevent the compiler from optimizing the kernel by
+ auto-inlining functions not marked with the inline keyword.
+ With this option, only functions explicitly marked with
+ "inline" will be inlined. This will allow the function tracer
+ to trace more functions because it only traces functions that
+ the compiler has not inlined.
+
+ Enabling this function can help debugging a kernel if using
+ the function tracer. But it can also change how the kernel
+ works, because inlining functions may change the timing,
+ which could make it difficult while debugging race conditions.
+
+ If unsure, select N.
+
+config ENABLE_WARN_DEPRECATED
+   bool "Enable __deprecated logic"
+   default y
+   help
+ Enable the __deprecated logic in the kernel build.
+ Disable this to suppress the "warning: 'foo' is deprecated
+ (declared at kernel/power/somefile.c:1234)" messages.
+
 config ENABLE_MUST_CHECK
bool "Enable __must_check logic"
default y
-- 
2.17.1

[PATCH 0/4] kernel hacking: GCC optimization for better debug experience (-Og)

[Du Changbin]

Hi all,
I have posted this series several months ago but interrupted by personal
affairs. Now I get time to complete this task. Thanks for all of the
reviewers.

I know some kernel developers was searching for a method to dissable GCC
optimizations, probably they want to apply GCC '-O0' option. But since Linux
kernel replys on GCC optimization to remove some dead code, so '-O0' just
breaks the build. They do need this because they want to debug kernel with
qemu, simics, kgtp or kgdb.

Thanks for the GCC '-Og' optimization level introduced in GCC 4.8, which
offers a reasonable level of optimization while maintaining fast compilation
and a good debugging experience. It is similar to '-O1' while perferring to
keep debug ability over runtime speed. With '-Og', we can build a kernel with
better debug ability and little performance drop after some simple change.

In this series, firstly introduce a new config CONFIG_NO_AUTO_INLINE after two
fixes for this new option. With this option, only functions explicitly marked
with "inline" will  be inlined. This will allow the function tracer to trace
more functions because it only traces functions that the compiler has not
inlined.

Then introduce new config CC_OPTIMIZE_FOR_DEBUGGING which apply '-Og'
optimization level for whole kernel, with a simple fix in fix_to_virt().
Currently I have only tested this option on x86 and ARM platform. Other
platforms should also work but probably need some compiling fixes as what
having done in this series. I leave that to who want to try this debug
option.

Comparison of vmlinux size: a bit smaller.

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
22665554   9709674  2920908 3529613621a9388 vmlinux

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ size vmlinux
   textdata bss dec hex filename
21499032   10102758 2920908 3452269820ec64a vmlinux


Comparison of system performance: a bit drop (~6%).
This benchmark of kernel compilation is suggested by Ingo Molnar.
https://lkml.org/lkml/2018/5/2/74

Preparation: Set cpufreq to 'performance'.
for ((cpu=0; cpu<120; cpu++)); do
  G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
  [ -f $G ] && echo performance > $G
done

w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

 Performance counter stats for 'make -j8' (5 runs):

219.764246652 seconds time elapsed   ( +-  0.78% )

w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
$ perf stat --repeat 5 --null --pre '\
cp -a kernel ../kernel.copy.$(date +%s); \
rm -rf *;\
git checkout .;  \
echo 1 > /proc/sys/vm/drop_caches;   \
find ../kernel* -type f | xargs cat >/dev/null;  \
make -j kernel >/dev/null;   \
make clean >/dev/null 2>&1;  \
sync'\
 \
make -j8 >/dev/null

Performance counter stats for 'make -j8' (5 runs):

 233.574187771 seconds time elapsed  ( +-  0.19% )


Changbin Du (4):
  x86/mm: surround level4_kernel_pgt with #ifdef
CONFIG_X86_5LEVEL...#endif
  kernel hacking: new config NO_AUTO_INLINE to disable compiler
auto-inline optimizations
  ARM: mm: fix build error in fix_to_virt with
CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
  kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og
optimization

 Makefile  | 11 +++
 arch/arm/mm/mmu.c |  2 +-
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c  | 13 ++---
 include/linux/compiler-gcc.h  |  2 +-
 include/linux/compiler.h  |  2 +-
 init/Kconfig  | 19 +++
 lib/Kconfig.debug | 25 +
 8 files changed, 66 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v3] scripts/gdb: fix lx-version

[Du Changbin]

For current gdb version (has tested with 7.3 and 8.1), 'lx-version'
only prints one character.
(gdb) lx-version
L(gdb)

This can be fixed by casting 'linux_banner' as (char *).
(gdb) lx-version
Linux version 4.19.0-rc1+ (changbin@acer) (gcc version 7.3.0 (Ubuntu 
7.3.0-16ubuntu3)) #21 SMP Sat Sep 1 21:43:30 CST 2018

Signed-off-by: Du Changbin 
---
 scripts/gdb/linux/proc.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
index 086d27223c0c..0aebd7565b03 100644
--- a/scripts/gdb/linux/proc.py
+++ b/scripts/gdb/linux/proc.py
@@ -41,7 +41,7 @@ class LxVersion(gdb.Command):
 
 def invoke(self, arg, from_tty):
 # linux_banner should contain a newline
-gdb.write(gdb.parse_and_eval("linux_banner").string())
+gdb.write(gdb.parse_and_eval("(char *)linux_banner").string())
 
 LxVersion()
 
-- 
2.17.1

[PATCH v3] scripts/gdb: fix lx-version

[Du Changbin]

For current gdb version (has tested with 7.3 and 8.1), 'lx-version'
only prints one character.
(gdb) lx-version
L(gdb)

This can be fixed by casting 'linux_banner' as (char *).
(gdb) lx-version
Linux version 4.19.0-rc1+ (changbin@acer) (gcc version 7.3.0 (Ubuntu 
7.3.0-16ubuntu3)) #21 SMP Sat Sep 1 21:43:30 CST 2018

Signed-off-by: Du Changbin 
---
 scripts/gdb/linux/proc.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
index 086d27223c0c..0aebd7565b03 100644
--- a/scripts/gdb/linux/proc.py
+++ b/scripts/gdb/linux/proc.py
@@ -41,7 +41,7 @@ class LxVersion(gdb.Command):
 
 def invoke(self, arg, from_tty):
 # linux_banner should contain a newline
-gdb.write(gdb.parse_and_eval("linux_banner").string())
+gdb.write(gdb.parse_and_eval("(char *)linux_banner").string())
 
 LxVersion()
 
-- 
2.17.1

Re: [PATCH] PCI: make pci_size() return real size

[Du Changbin]

Hi Bjorn. Have you checked this little improvment? The idea here is that
this is not a hotspot, so readbility matters than trick. Thanks!

On Sat, Oct 13, 2018 at 08:49:19AM +0800, changbin...@gmail.com wrote:
> From: Du Changbin 
> 
> Currently, the pci_size() function actually return 'size-1'.
> Make it return real size to avoid confusing.
> 
> Signed-off-by: Du Changbin 
> ---
>  drivers/pci/probe.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 201f9e5ff55c..8ff2b1413865 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -121,13 +121,13 @@ static u64 pci_size(u64 base, u64 maxbase, u64 mask)
>* Get the lowest of them to find the decode size, and from that
>* the extent.
>*/
> - size = (size & ~(size-1)) - 1;
> + size = size & ~(size-1);
>  
>   /*
>* base == maxbase can be valid only if the BAR has already been
>* programmed with all 1s.
>*/
> - if (base == maxbase && ((base | size) & mask) != mask)
> + if (base == maxbase && ((base | (size - 1)) & mask) != mask)
>   return 0;
>  
>   return size;
> @@ -278,7 +278,7 @@ int __pci_read_base(struct pci_dev *dev, enum 
> pci_bar_type type,
>   /* Above 32-bit boundary; try to reallocate */
>   res->flags |= IORESOURCE_UNSET;
>   res->start = 0;
> - res->end = sz64;
> + res->end = sz64 - 1;
>   pci_info(dev, "reg 0x%x: can't handle BAR above 4GB 
> (bus address %#010llx)\n",
>pos, (unsigned long long)l64);
>   goto out;
> @@ -286,7 +286,7 @@ int __pci_read_base(struct pci_dev *dev, enum 
> pci_bar_type type,
>   }
>  
>   region.start = l64;
> - region.end = l64 + sz64;
> + region.end = l64 + sz64 - 1;
>  
>   pcibios_bus_to_resource(dev->bus, res, );
>   pcibios_resource_to_bus(dev->bus, _region, res);
> -- 
> 2.17.1
> 

-- 
Thanks,
Du Changbin

Re: [PATCH] PCI: make pci_size() return real size

[Du Changbin]

Hi Bjorn. Have you checked this little improvment? The idea here is that
this is not a hotspot, so readbility matters than trick. Thanks!

On Sat, Oct 13, 2018 at 08:49:19AM +0800, changbin...@gmail.com wrote:
> From: Du Changbin 
> 
> Currently, the pci_size() function actually return 'size-1'.
> Make it return real size to avoid confusing.
> 
> Signed-off-by: Du Changbin 
> ---
>  drivers/pci/probe.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 201f9e5ff55c..8ff2b1413865 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -121,13 +121,13 @@ static u64 pci_size(u64 base, u64 maxbase, u64 mask)
>* Get the lowest of them to find the decode size, and from that
>* the extent.
>*/
> - size = (size & ~(size-1)) - 1;
> + size = size & ~(size-1);
>  
>   /*
>* base == maxbase can be valid only if the BAR has already been
>* programmed with all 1s.
>*/
> - if (base == maxbase && ((base | size) & mask) != mask)
> + if (base == maxbase && ((base | (size - 1)) & mask) != mask)
>   return 0;
>  
>   return size;
> @@ -278,7 +278,7 @@ int __pci_read_base(struct pci_dev *dev, enum 
> pci_bar_type type,
>   /* Above 32-bit boundary; try to reallocate */
>   res->flags |= IORESOURCE_UNSET;
>   res->start = 0;
> - res->end = sz64;
> + res->end = sz64 - 1;
>   pci_info(dev, "reg 0x%x: can't handle BAR above 4GB 
> (bus address %#010llx)\n",
>pos, (unsigned long long)l64);
>   goto out;
> @@ -286,7 +286,7 @@ int __pci_read_base(struct pci_dev *dev, enum 
> pci_bar_type type,
>   }
>  
>   region.start = l64;
> - region.end = l64 + sz64;
> + region.end = l64 + sz64 - 1;
>  
>   pcibios_bus_to_resource(dev->bus, res, );
>   pcibios_resource_to_bus(dev->bus, _region, res);
> -- 
> 2.17.1
> 

-- 
Thanks,
Du Changbin

[PATCH v2] scripts/gdb: fix lx-version for gdb 7.3-

[Du Changbin]

For gdb version less than 7.3, lx-version only prints one character.
(gdb) lx-version
L(gdb)

This can be fixed by casting 'linux_banner' as (char *).
(gdb) lx-version
Linux version 4.19.0-rc1+ (changbin@acer) (gcc version 7.3.0 (Ubuntu 
7.3.0-16ubuntu3)) #21 SMP Sat Sep 1 21:43:30 CST 2018

gdb 7.4 seems to be no such issue.

Signed-off-by: Du Changbin 
---
 scripts/gdb/linux/proc.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
index 086d27223c0c..0aebd7565b03 100644
--- a/scripts/gdb/linux/proc.py
+++ b/scripts/gdb/linux/proc.py
@@ -41,7 +41,7 @@ class LxVersion(gdb.Command):
 
 def invoke(self, arg, from_tty):
 # linux_banner should contain a newline
-gdb.write(gdb.parse_and_eval("linux_banner").string())
+gdb.write(gdb.parse_and_eval("(char *)linux_banner").string())
 
 LxVersion()
 
-- 
2.17.1

[PATCH v2] scripts/gdb: fix lx-version for gdb 7.3-

[Du Changbin]

For gdb version less than 7.3, lx-version only prints one character.
(gdb) lx-version
L(gdb)

This can be fixed by casting 'linux_banner' as (char *).
(gdb) lx-version
Linux version 4.19.0-rc1+ (changbin@acer) (gcc version 7.3.0 (Ubuntu 
7.3.0-16ubuntu3)) #21 SMP Sat Sep 1 21:43:30 CST 2018

gdb 7.4 seems to be no such issue.

Signed-off-by: Du Changbin 
---
 scripts/gdb/linux/proc.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
index 086d27223c0c..0aebd7565b03 100644
--- a/scripts/gdb/linux/proc.py
+++ b/scripts/gdb/linux/proc.py
@@ -41,7 +41,7 @@ class LxVersion(gdb.Command):
 
 def invoke(self, arg, from_tty):
 # linux_banner should contain a newline
-gdb.write(gdb.parse_and_eval("linux_banner").string())
+gdb.write(gdb.parse_and_eval("(char *)linux_banner").string())
 
 LxVersion()
 
-- 
2.17.1

[PATCH] scripts/gdb: fix lx-version for gdb 7.3-

[Du Changbin]

For gdb version less than 7.3, lx-version only one character.
(gdb) lx-version
L(gdb)

This can be fixed by casting 'linux_banner' as (char *).
(gdb) lx-version
Linux version 4.19.0-rc1+ (changbin@acer) (gcc version 7.3.0 (Ubuntu 
7.3.0-16ubuntu3)) #21 SMP Sat Sep 1 21:43:30 CST 2018

gdb 7.4 seems to be no such issue.

Signed-off-by: Du Changbin 
---
 scripts/gdb/linux/proc.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
index 086d27223c0c..0aebd7565b03 100644
--- a/scripts/gdb/linux/proc.py
+++ b/scripts/gdb/linux/proc.py
@@ -41,7 +41,7 @@ class LxVersion(gdb.Command):
 
 def invoke(self, arg, from_tty):
 # linux_banner should contain a newline
-gdb.write(gdb.parse_and_eval("linux_banner").string())
+gdb.write(gdb.parse_and_eval("(char *)linux_banner").string())
 
 LxVersion()
 
-- 
2.17.1

[PATCH] scripts/gdb: fix lx-version for gdb 7.3-

[Du Changbin]

For gdb version less than 7.3, lx-version only one character.
(gdb) lx-version
L(gdb)

This can be fixed by casting 'linux_banner' as (char *).
(gdb) lx-version
Linux version 4.19.0-rc1+ (changbin@acer) (gcc version 7.3.0 (Ubuntu 
7.3.0-16ubuntu3)) #21 SMP Sat Sep 1 21:43:30 CST 2018

gdb 7.4 seems to be no such issue.

Signed-off-by: Du Changbin 
---
 scripts/gdb/linux/proc.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
index 086d27223c0c..0aebd7565b03 100644
--- a/scripts/gdb/linux/proc.py
+++ b/scripts/gdb/linux/proc.py
@@ -41,7 +41,7 @@ class LxVersion(gdb.Command):
 
 def invoke(self, arg, from_tty):
 # linux_banner should contain a newline
-gdb.write(gdb.parse_and_eval("linux_banner").string())
+gdb.write(gdb.parse_and_eval("(char *)linux_banner").string())
 
 LxVersion()
 
-- 
2.17.1

Re: kernel BUG at arch/x86/kvm/x86.c:LINE! (2)

[Du Changbin]

vm_spurious_fault+0x9/0x10 arch/x86/kvm/x86.c:353
Code: 45 10 50 e8 e9 44 7c 00 58 5a 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f
5d c3 0f 1f 84 00 00 00 00 00 55 48 89 e5 e8 97 03 73 00 <0f> 0b 0f 1f 44 00
00 55 48 89 e5 41 57 41 56 41 55 41 89 fd 41 54
RSP: 0018:88018b397448 EFLAGS: 00010212
RAX: 0004 RBX: 110031672e8d RCX: c9000628e000
RDX: 0417 RSI: 810bd1f9 RDI: 88018b397488
RBP: 88018b397448 R08: 8801cc2f2180 R09: 8801c308d000
R10: ed0038611bff R11: 8801c308dfff R12: 88018b3974c8
R13: dc00 R14: 8801c308d000 R15: 88018b397488
 kvm_fastop_exception+0x50b/0x5455
 loaded_vmcs_init arch/x86/kvm/vmx.c:2129 [inline]
 alloc_loaded_vmcs+0x7f/0x280 arch/x86/kvm/vmx.c:4766
 vmx_create_vcpu+0x20e/0x25e0 arch/x86/kvm/vmx.c:11025
 kvm_arch_vcpu_create+0xe5/0x220 arch/x86/kvm/x86.c:8471
 kvm_vm_ioctl_create_vcpu arch/x86/kvm/../../../virt/kvm/kvm_main.c:2476
[inline]
 kvm_vm_ioctl+0x470/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2977
 kvm_vm_compat_ioctl+0x143/0x430
arch/x86/kvm/../../../virt/kvm/kvm_main.c:3170
 __do_compat_sys_ioctl fs/compat_ioctl.c:1419 [inline]
 __se_compat_sys_ioctl fs/compat_ioctl.c:1365 [inline]
 __ia32_compat_sys_ioctl+0x20e/0x630 fs/compat_ioctl.c:1365
 do_syscall_32_irqs_on arch/x86/entry/common.c:326 [inline]
 do_fast_syscall_32+0x34d/0xfb2 arch/x86/entry/common.c:397
 entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7fecca9
Code: 85 d2 74 02 89 0a 5b 5d c3 8b 04 24 c3 8b 0c 24 c3 8b 1c 24 c3 90 90
90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90
90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:f5fa60cc EFLAGS: 0296 ORIG_RAX: 0036
RAX: ffda RBX: 000c RCX: ae41
RDX:  RSI:  RDI: 
RBP:  R08:  R09: 
R10:  R11:  R12: 
R13:  R14:  R15: 
Modules linked in:
---[ end trace 1c8fec48833612c0 ]---
RIP: 0010:kvm_spurious_fault+0x9/0x10 arch/x86/kvm/x86.c:353
Code: 45 10 50 e8 e9 44 7c 00 58 5a 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f
5d c3 0f 1f 84 00 00 00 00 00 55 48 89 e5 e8 97 03 73 00 <0f> 0b 0f 1f 44 00
00 55 48 89 e5 41 57 41 56 41 55 41 89 fd 41 54
RSP: 0018:8801dae07bd8 EFLAGS: 00010006
RAX: 8801cc2f2180 RBX: 11003b5c0f7f RCX: 81385bcc
RDX: 0001 RSI: 810bd1f9 RDI: 8801dae07c18
RBP: 8801dae07bd8 R08: 8801cc2f2180 R09: ed003b5c5ba0
R10: ed003b5c5ba0 R11: 8801dae2dd07 R12: 8801dae07c58
R13: dc00 R14: 8801beccc000 R15: 8801dae07c18
FS:  () GS:8801dae0(0063) knlGS:f5fa6b40
CS:  0010 DS: 002b ES: 002b CR0: 80050033
CR2: 8801dae07c18 CR3: 0001cc8d1000 CR4: 001426f0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkal...@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.


--
Thanks,
Du Changbin

Re: kernel BUG at arch/x86/kvm/x86.c:LINE! (2)

[Du Changbin]

vm_spurious_fault+0x9/0x10 arch/x86/kvm/x86.c:353
Code: 45 10 50 e8 e9 44 7c 00 58 5a 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f
5d c3 0f 1f 84 00 00 00 00 00 55 48 89 e5 e8 97 03 73 00 <0f> 0b 0f 1f 44 00
00 55 48 89 e5 41 57 41 56 41 55 41 89 fd 41 54
RSP: 0018:88018b397448 EFLAGS: 00010212
RAX: 0004 RBX: 110031672e8d RCX: c9000628e000
RDX: 0417 RSI: 810bd1f9 RDI: 88018b397488
RBP: 88018b397448 R08: 8801cc2f2180 R09: 8801c308d000
R10: ed0038611bff R11: 8801c308dfff R12: 88018b3974c8
R13: dc00 R14: 8801c308d000 R15: 88018b397488
 kvm_fastop_exception+0x50b/0x5455
 loaded_vmcs_init arch/x86/kvm/vmx.c:2129 [inline]
 alloc_loaded_vmcs+0x7f/0x280 arch/x86/kvm/vmx.c:4766
 vmx_create_vcpu+0x20e/0x25e0 arch/x86/kvm/vmx.c:11025
 kvm_arch_vcpu_create+0xe5/0x220 arch/x86/kvm/x86.c:8471
 kvm_vm_ioctl_create_vcpu arch/x86/kvm/../../../virt/kvm/kvm_main.c:2476
[inline]
 kvm_vm_ioctl+0x470/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2977
 kvm_vm_compat_ioctl+0x143/0x430
arch/x86/kvm/../../../virt/kvm/kvm_main.c:3170
 __do_compat_sys_ioctl fs/compat_ioctl.c:1419 [inline]
 __se_compat_sys_ioctl fs/compat_ioctl.c:1365 [inline]
 __ia32_compat_sys_ioctl+0x20e/0x630 fs/compat_ioctl.c:1365
 do_syscall_32_irqs_on arch/x86/entry/common.c:326 [inline]
 do_fast_syscall_32+0x34d/0xfb2 arch/x86/entry/common.c:397
 entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7fecca9
Code: 85 d2 74 02 89 0a 5b 5d c3 8b 04 24 c3 8b 0c 24 c3 8b 1c 24 c3 90 90
90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90
90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:f5fa60cc EFLAGS: 0296 ORIG_RAX: 0036
RAX: ffda RBX: 000c RCX: ae41
RDX:  RSI:  RDI: 
RBP:  R08:  R09: 
R10:  R11:  R12: 
R13:  R14:  R15: 
Modules linked in:
---[ end trace 1c8fec48833612c0 ]---
RIP: 0010:kvm_spurious_fault+0x9/0x10 arch/x86/kvm/x86.c:353
Code: 45 10 50 e8 e9 44 7c 00 58 5a 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f
5d c3 0f 1f 84 00 00 00 00 00 55 48 89 e5 e8 97 03 73 00 <0f> 0b 0f 1f 44 00
00 55 48 89 e5 41 57 41 56 41 55 41 89 fd 41 54
RSP: 0018:8801dae07bd8 EFLAGS: 00010006
RAX: 8801cc2f2180 RBX: 11003b5c0f7f RCX: 81385bcc
RDX: 0001 RSI: 810bd1f9 RDI: 8801dae07c18
RBP: 8801dae07bd8 R08: 8801cc2f2180 R09: ed003b5c5ba0
R10: ed003b5c5ba0 R11: 8801dae2dd07 R12: 8801dae07c58
R13: dc00 R14: 8801beccc000 R15: 8801dae07c18
FS:  () GS:8801dae0(0063) knlGS:f5fa6b40
CS:  0010 DS: 002b ES: 002b CR0: 80050033
CR2: 8801dae07c18 CR3: 0001cc8d1000 CR4: 001426f0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkal...@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.


--
Thanks,
Du Changbin

Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du, Changbin]

Hi,
On Thu, Jun 07, 2018 at 09:47:18AM +0530, Viresh Kumar wrote:
> +Greg/Alex,
> 
> @Fegguang/build-bot: I do see mention of Greg and /me in your initial email's
> body saying TO: Viresh, CC: Greg, but I don't see any of us getting cc'd in 
> your
> email. Bug ?
> 
> On 06-06-18, 14:26, Steven Rostedt wrote:
> > On Wed, 6 Jun 2018 16:26:00 +0200
> > Johan Hovold  wrote:
> > 
> > > Looks like the greybus code above is working as intended by checking for
> > > unterminated string after the strncpy, even if this does now triggers
> > > the truncation warning.
> 
> So why exactly are we generating a warning here ? Is it because it is possible
> that the first n bytes of src may not have the null terminating byte and the
> dest may not be null terminated eventually ?
> 
> Maybe I should just use memcpy here then ?
> 
I think if the destination is not a null terminated string (If I understand your
description below), memcpy can be used to get rid of such warning. The warning
makes sense in general as explained in mannual. Thanks!

> But AFAIR, I used strncpy() specifically because it also sets all the 
> remaining
> bytes after the null terminating byte with the null terminating byte. And so 
> it
> is pretty easy for me to check if the final string is null terminated by
> checking [max - 1] byte against '\0', which the code is doing right now.
> 
> I am not sure what would the best way to get around this incorrect-warning.
> 
> And I am wondering on why buildbot reported the warning only for two instances
> in that file, while I have done the same thing at 4 places.
> 
> > Ah, yes I now see that. Thanks for pointing it out. But perhaps it
> > should also add the "- 1" to the strncpy() so that gcc doesn't think
> > it's a mistake.
> 
> The src string is passed on from a firmware entity and we need to make sure 
> the
> protocol (greybus) is implemented properly by the other end. For example, in 
> the
> current case if the firmware sends "HELLOWORLD", its an error as it should 
> have
> sent "HELLWORLD\0". But with what you are saying we will forcefully make dest 
> as
> "HELLWORLD\0", which wouldn't be the right thing to do as we will miss the bug
> present in firmware.
> 
> -- 
> viresh

-- 
Thanks,
Changbin Du

Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du, Changbin]

Hi,
On Thu, Jun 07, 2018 at 09:47:18AM +0530, Viresh Kumar wrote:
> +Greg/Alex,
> 
> @Fegguang/build-bot: I do see mention of Greg and /me in your initial email's
> body saying TO: Viresh, CC: Greg, but I don't see any of us getting cc'd in 
> your
> email. Bug ?
> 
> On 06-06-18, 14:26, Steven Rostedt wrote:
> > On Wed, 6 Jun 2018 16:26:00 +0200
> > Johan Hovold  wrote:
> > 
> > > Looks like the greybus code above is working as intended by checking for
> > > unterminated string after the strncpy, even if this does now triggers
> > > the truncation warning.
> 
> So why exactly are we generating a warning here ? Is it because it is possible
> that the first n bytes of src may not have the null terminating byte and the
> dest may not be null terminated eventually ?
> 
> Maybe I should just use memcpy here then ?
> 
I think if the destination is not a null terminated string (If I understand your
description below), memcpy can be used to get rid of such warning. The warning
makes sense in general as explained in mannual. Thanks!

> But AFAIR, I used strncpy() specifically because it also sets all the 
> remaining
> bytes after the null terminating byte with the null terminating byte. And so 
> it
> is pretty easy for me to check if the final string is null terminated by
> checking [max - 1] byte against '\0', which the code is doing right now.
> 
> I am not sure what would the best way to get around this incorrect-warning.
> 
> And I am wondering on why buildbot reported the warning only for two instances
> in that file, while I have done the same thing at 4 places.
> 
> > Ah, yes I now see that. Thanks for pointing it out. But perhaps it
> > should also add the "- 1" to the strncpy() so that gcc doesn't think
> > it's a mistake.
> 
> The src string is passed on from a firmware entity and we need to make sure 
> the
> protocol (greybus) is implemented properly by the other end. For example, in 
> the
> current case if the firmware sends "HELLOWORLD", its an error as it should 
> have
> sent "HELLWORLD\0". But with what you are saying we will forcefully make dest 
> as
> "HELLWORLD\0", which wouldn't be the right thing to do as we will miss the bug
> present in firmware.
> 
> -- 
> viresh

-- 
Thanks,
Changbin Du

Re: [PATCH] scripts/faddr2line: show the code context

[Du, Changbin]

On Tue, May 29, 2018 at 06:03:32PM +0200, Peter Zijlstra wrote:
> On Mon, Mar 19, 2018 at 03:23:25PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > Inspired by gdb command 'list', show the code context of target lines.
> > Here is a example:
> > 
> > $ scripts/faddr2line vmlinux native_write_msr+0x6
> > native_write_msr+0x6/0x20:
> > arch_static_branch at arch/x86/include/asm/msr.h:105
> > 100 return EAX_EDX_VAL(val, low, high);
> > 101 }
> > 102
> > 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 
> > high)
> > 104 {
> > 105 asm volatile("1: wrmsr\n"
> > 106  "2:\n"
> > 107  _ASM_EXTABLE_HANDLE(1b, 2b, 
> > ex_handler_wrmsr_unsafe)
> > 108  : : "c" (msr), "a"(low), "d" (high) : 
> > "memory");
> > 109 }
> > 110
> > (inlined by) static_key_false at include/linux/jump_label.h:142
> > 137 #define JUMP_TYPE_LINKED2UL
> > 138 #define JUMP_TYPE_MASK  3UL
> > 139
> > 140 static __always_inline bool static_key_false(struct static_key *key)
> > 141 {
> > 142 return arch_static_branch(key, false);
> > 143 }
> > 144
> > 145 static __always_inline bool static_key_true(struct static_key *key)
> > 146 {
> > 147 return !arch_static_branch(key, true);
> > (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> > 145 static inline void notrace
> > 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> > 147 {
> > 148 __wrmsr(msr, low, high);
> > 149
> > 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> > 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> > 152 }
> > 153
> > 154 /* Can be uninlined because referenced by paravirt */
> > 155 static inline int notrace
> 
> Not a fan of this :-/ And you didn't even make it optional. Nor did you
> Cc the original author of the tool.
Yeah, understand your compatibility concern, and thanks for your improvment.
I only added people from 'scripts/get_maintainer.pl'.


-- 
Thanks,
Changbin Du

Re: [PATCH] scripts/faddr2line: show the code context

[Du, Changbin]

On Tue, May 29, 2018 at 06:03:32PM +0200, Peter Zijlstra wrote:
> On Mon, Mar 19, 2018 at 03:23:25PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > Inspired by gdb command 'list', show the code context of target lines.
> > Here is a example:
> > 
> > $ scripts/faddr2line vmlinux native_write_msr+0x6
> > native_write_msr+0x6/0x20:
> > arch_static_branch at arch/x86/include/asm/msr.h:105
> > 100 return EAX_EDX_VAL(val, low, high);
> > 101 }
> > 102
> > 103 static inline void notrace __wrmsr(unsigned int msr, u32 low, u32 
> > high)
> > 104 {
> > 105 asm volatile("1: wrmsr\n"
> > 106  "2:\n"
> > 107  _ASM_EXTABLE_HANDLE(1b, 2b, 
> > ex_handler_wrmsr_unsafe)
> > 108  : : "c" (msr), "a"(low), "d" (high) : 
> > "memory");
> > 109 }
> > 110
> > (inlined by) static_key_false at include/linux/jump_label.h:142
> > 137 #define JUMP_TYPE_LINKED2UL
> > 138 #define JUMP_TYPE_MASK  3UL
> > 139
> > 140 static __always_inline bool static_key_false(struct static_key *key)
> > 141 {
> > 142 return arch_static_branch(key, false);
> > 143 }
> > 144
> > 145 static __always_inline bool static_key_true(struct static_key *key)
> > 146 {
> > 147 return !arch_static_branch(key, true);
> > (inlined by) native_write_msr at arch/x86/include/asm/msr.h:150
> > 145 static inline void notrace
> > 146 native_write_msr(unsigned int msr, u32 low, u32 high)
> > 147 {
> > 148 __wrmsr(msr, low, high);
> > 149
> > 150 if (msr_tracepoint_active(__tracepoint_write_msr))
> > 151 do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
> > 152 }
> > 153
> > 154 /* Can be uninlined because referenced by paravirt */
> > 155 static inline int notrace
> 
> Not a fan of this :-/ And you didn't even make it optional. Nor did you
> Cc the original author of the tool.
Yeah, understand your compatibility concern, and thanks for your improvment.
I only added people from 'scripts/get_maintainer.pl'.


-- 
Thanks,
Changbin Du

Re: [PATCH] scripts/faddr2line: show the code context

[Du, Changbin]

On Wed, May 30, 2018 at 08:01:48AM +1000, NeilBrown wrote:
> On Tue, May 29 2018, Peter Zijlstra wrote:
> 
> > On Tue, May 29, 2018 at 12:07:10PM -0500, Josh Poimboeuf wrote:
> >> Yeah, this change really should have been an optional arg.  It hurt the
> >> readability and compactness of the output.  The above looks good to me.
> >> 
> >> Care to send a proper patch?  If you send it to Linus he might apply it
> >> directly as he did with my original patches.
> >
> > ---
> > From: Peter Zijlstra (Intel) 
> >
> > Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > radically altered the output format of the faddr2line tool. And while
> > the new list output format might have merrit it broke my vim usage and
> > was hard to read.
> >
> > Make the new format optional; using a '--list' argument and attempt to
> > make the output slightly easier to read by adding a little whitespace to
> > separate the different files and explicitly mark the line in question.
> 
> Not commenting on your code but on the original patch.
> I've recently noticed that ADDR2LINE sometimes outputs
>   (discriminator 2)
> or similar at the end of the line.  This messes up the parsing.
> 
> I hacked it to work so I could keep debugging with
> 
> - local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; 
> $dir_prefix\(\./\)*; ;")
> + local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed -e 
> "s; $dir_prefix\(\./\)*; ;" -e "s/(discriminator [0-9]*)//")
> 
> but someone should probably find out exactly what sort of messages
> ADDR2LINE produces, and make sure they are all parsed correctly.
> (maybe that someone should be me, but not today).
> 
Hi, I have fixed it by commit 78eb0c6356 ("scripts/faddr2line: fix error when
addr2line output contains discriminator") and it is already in the mainline now.
Thank you!

> Thanks,
> NeilBrown
> 
> 
> >
> > Cc: Changbin Du 
> > Acked-by: Josh Poimboeuf 
> > Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > Signed-off-by: Peter Zijlstra (Intel) 
> > ---
> >  scripts/faddr2line | 18 --
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/faddr2line b/scripts/faddr2line
> > index 1876a741087c..a0149db00be7 100755
> > --- a/scripts/faddr2line
> > +++ b/scripts/faddr2line
> > @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't 
> > installed"
> >  command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
> >  
> >  usage() {
> > -   echo "usage: faddr2line   ..." 
> > >&2
> > +   echo "usage: faddr2line [--list]   
> > ..." >&2
> > exit 1
> >  }
> >  
> > @@ -166,15 +166,25 @@ __faddr2line() {
> > local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; 
> > $dir_prefix\(\./\)*; ;")
> > [[ -z $file_lines ]] && return
> >  
> > +   if [[ $LIST = 0 ]]; then
> > +   echo "$file_lines" | while read -r line
> > +   do
> > +   echo $line
> > +   done
> > +   DONE=1;
> > +   return
> > +   fi
> > +
> > # show each line with context
> > echo "$file_lines" | while read -r line
> > do
> > +   echo
> > echo $line
> > n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> > n1=$[$n-5]
> > n2=$[$n+5]
> > f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> > -   awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") 
> > {printf("%d\t%s\n", NR, $0)}' $f
> > +   awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { 
> > if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", 
> > $0)}' $f
> > done
> >  
> > DONE=1
> > @@ -185,6 +195,10 @@ __faddr2line() {
> >  [[ $# -lt 2 ]] && usage
> >  
> >  objfile=$1
> > +
> > +LIST=0
> > +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> > +
> >  [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> >  shift
> >  



-- 
Thanks,
Changbin Du

Re: [PATCH] scripts/faddr2line: show the code context

[Du, Changbin]

On Wed, May 30, 2018 at 08:01:48AM +1000, NeilBrown wrote:
> On Tue, May 29 2018, Peter Zijlstra wrote:
> 
> > On Tue, May 29, 2018 at 12:07:10PM -0500, Josh Poimboeuf wrote:
> >> Yeah, this change really should have been an optional arg.  It hurt the
> >> readability and compactness of the output.  The above looks good to me.
> >> 
> >> Care to send a proper patch?  If you send it to Linus he might apply it
> >> directly as he did with my original patches.
> >
> > ---
> > From: Peter Zijlstra (Intel) 
> >
> > Commit 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > radically altered the output format of the faddr2line tool. And while
> > the new list output format might have merrit it broke my vim usage and
> > was hard to read.
> >
> > Make the new format optional; using a '--list' argument and attempt to
> > make the output slightly easier to read by adding a little whitespace to
> > separate the different files and explicitly mark the line in question.
> 
> Not commenting on your code but on the original patch.
> I've recently noticed that ADDR2LINE sometimes outputs
>   (discriminator 2)
> or similar at the end of the line.  This messes up the parsing.
> 
> I hacked it to work so I could keep debugging with
> 
> - local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; 
> $dir_prefix\(\./\)*; ;")
> + local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed -e 
> "s; $dir_prefix\(\./\)*; ;" -e "s/(discriminator [0-9]*)//")
> 
> but someone should probably find out exactly what sort of messages
> ADDR2LINE produces, and make sure they are all parsed correctly.
> (maybe that someone should be me, but not today).
> 
Hi, I have fixed it by commit 78eb0c6356 ("scripts/faddr2line: fix error when
addr2line output contains discriminator") and it is already in the mainline now.
Thank you!

> Thanks,
> NeilBrown
> 
> 
> >
> > Cc: Changbin Du 
> > Acked-by: Josh Poimboeuf 
> > Fixes: 6870c0165feaa5 ("scripts/faddr2line: show the code context")
> > Signed-off-by: Peter Zijlstra (Intel) 
> > ---
> >  scripts/faddr2line | 18 --
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/faddr2line b/scripts/faddr2line
> > index 1876a741087c..a0149db00be7 100755
> > --- a/scripts/faddr2line
> > +++ b/scripts/faddr2line
> > @@ -56,7 +56,7 @@ command -v ${SIZE} >/dev/null 2>&1 || die "size isn't 
> > installed"
> >  command -v ${NM} >/dev/null 2>&1 || die "nm isn't installed"
> >  
> >  usage() {
> > -   echo "usage: faddr2line   ..." 
> > >&2
> > +   echo "usage: faddr2line [--list]   
> > ..." >&2
> > exit 1
> >  }
> >  
> > @@ -166,15 +166,25 @@ __faddr2line() {
> > local file_lines=$(${ADDR2LINE} -fpie $objfile $addr | sed "s; 
> > $dir_prefix\(\./\)*; ;")
> > [[ -z $file_lines ]] && return
> >  
> > +   if [[ $LIST = 0 ]]; then
> > +   echo "$file_lines" | while read -r line
> > +   do
> > +   echo $line
> > +   done
> > +   DONE=1;
> > +   return
> > +   fi
> > +
> > # show each line with context
> > echo "$file_lines" | while read -r line
> > do
> > +   echo
> > echo $line
> > n=$(echo $line | sed 's/.*:\([0-9]\+\).*/\1/g')
> > n1=$[$n-5]
> > n2=$[$n+5]
> > f=$(echo $line | sed 's/.*at \(.\+\):.*/\1/g')
> > -   awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") 
> > {printf("%d\t%s\n", NR, $0)}' $f
> > +   awk 'NR>=strtonum("'$n1'") && NR<=strtonum("'$n2'") { 
> > if (NR=='$n') printf(">%d<", NR); else printf(" %d ", NR); printf("\t%s\n", 
> > $0)}' $f
> > done
> >  
> > DONE=1
> > @@ -185,6 +195,10 @@ __faddr2line() {
> >  [[ $# -lt 2 ]] && usage
> >  
> >  objfile=$1
> > +
> > +LIST=0
> > +[[ "$objfile" == "--list" ]] && LIST=1 && shift && objfile=$1
> > +
> >  [[ ! -f $objfile ]] && die "can't find objfile $objfile"
> >  shift
> >  



-- 
Thanks,
Changbin Du

Re: [PATCH v4 4/4] asm-generic: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING

[Du, Changbin]

On Wed, May 09, 2018 at 08:52:24AM -0400, Steven Rostedt wrote:
> On Wed,  9 May 2018 16:43:16 +0800
> changbin...@intel.com wrote:
> 
> > From: Changbin Du <changbin...@intel.com>
> > 
> > With '-Og' optimization level, GCC would not optimize a count for a loop
> > as a constant value. But BUILD_BUG_ON() only accept compile-time constant
> > values. Let's use __fix_to_virt() to avoid the error.
> > 
> > arch/arm/mm/mmu.o: In function `fix_to_virt':
> > /home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined 
> > reference to `__compiletime_assert_31'
> > Makefile:1051: recipe for target 'vmlinux' failed
> > make: *** [vmlinux] Error 1
> 
> Perhaps we should put this patch ahead of patch 3. Why allow it to
> break?
> 
Agree, let me exchange the last two patches.

> Anyway, besides that, I think the series looks good.
> 
> For the series: Acked-by: Steven Rostedt (VMware) <rost...@goodmis.org>
> 
> -- Steve
> 
> 
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > 
> > ---
> > v2: use __fix_to_virt() to fix the issue.
> > ---
> >  arch/arm/mm/mmu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > index e46a6a4..c08d74e 100644
> > --- a/arch/arm/mm/mmu.c
> > +++ b/arch/arm/mm/mmu.c
> > @@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
> > pte_t *pte;
> > struct map_desc map;
> >  
> > -   map.virtual = fix_to_virt(i);
> > +   map.virtual = __fix_to_virt(i);
> > pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), 
> > map.virtual);
> >  
> > /* Only i/o device mappings are supported ATM */
> 

-- 
Thanks,
Changbin Du

Re: [PATCH v4 4/4] asm-generic: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING

[Du, Changbin]

On Wed, May 09, 2018 at 08:52:24AM -0400, Steven Rostedt wrote:
> On Wed,  9 May 2018 16:43:16 +0800
> changbin...@intel.com wrote:
> 
> > From: Changbin Du 
> > 
> > With '-Og' optimization level, GCC would not optimize a count for a loop
> > as a constant value. But BUILD_BUG_ON() only accept compile-time constant
> > values. Let's use __fix_to_virt() to avoid the error.
> > 
> > arch/arm/mm/mmu.o: In function `fix_to_virt':
> > /home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined 
> > reference to `__compiletime_assert_31'
> > Makefile:1051: recipe for target 'vmlinux' failed
> > make: *** [vmlinux] Error 1
> 
> Perhaps we should put this patch ahead of patch 3. Why allow it to
> break?
> 
Agree, let me exchange the last two patches.

> Anyway, besides that, I think the series looks good.
> 
> For the series: Acked-by: Steven Rostedt (VMware) 
> 
> -- Steve
> 
> 
> > 
> > Signed-off-by: Changbin Du 
> > 
> > ---
> > v2: use __fix_to_virt() to fix the issue.
> > ---
> >  arch/arm/mm/mmu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > index e46a6a4..c08d74e 100644
> > --- a/arch/arm/mm/mmu.c
> > +++ b/arch/arm/mm/mmu.c
> > @@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
> > pte_t *pte;
> > struct map_desc map;
> >  
> > -   map.virtual = fix_to_virt(i);
> > +   map.virtual = __fix_to_virt(i);
> > pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), 
> > map.virtual);
> >  
> > /* Only i/o device mappings are supported ATM */
> 

-- 
Thanks,
Changbin Du

Re: [PATCH v3 2/5] regulator: add dummy function of_find_regulator_by_node

[Du, Changbin]

On Wed, May 09, 2018 at 05:21:14PM +0900, Mark Brown wrote:
> On Sun, May 06, 2018 at 08:20:13AM +0800, changbin...@intel.com wrote:
> > From: Changbin Du <changbin...@intel.com>
> > 
> > If device tree is not enabled, of_find_regulator_by_node() should have
> > a dummy function since the function call is still there.
> 
> Please do not submit new versions of already applied patches, please
> submit incremental updates to the existing code.  Modifying existing
> commits creates problems for other users building on top of those
> commits so it's best practice to only change pubished git commits if
> absolutely essential.

Hmm, I saw your merging notification too late. Let me refresh the series. Sorry
for confusing.

-- 
Thanks,
Changbin Du

Re: [PATCH v3 2/5] regulator: add dummy function of_find_regulator_by_node

[Du, Changbin]

On Wed, May 09, 2018 at 05:21:14PM +0900, Mark Brown wrote:
> On Sun, May 06, 2018 at 08:20:13AM +0800, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > If device tree is not enabled, of_find_regulator_by_node() should have
> > a dummy function since the function call is still there.
> 
> Please do not submit new versions of already applied patches, please
> submit incremental updates to the existing code.  Modifying existing
> commits creates problems for other users building on top of those
> commits so it's best practice to only change pubished git commits if
> absolutely essential.

Hmm, I saw your merging notification too late. Let me refresh the series. Sorry
for confusing.

-- 
Thanks,
Changbin Du

Re: [PATCH v2 4/5] kernel hacking: new config DEBUG_EXPERIENCE to apply GCC -Og optimization

[Du, Changbin]

On Thu, May 03, 2018 at 10:28:23AM -0400, Steven Rostedt wrote:
> On Thu, 3 May 2018 21:45:46 +0800
> "Du, Changbin" <changbin...@intel.com> wrote:
> 
> > > With that gcc comment, I still think CONFIG_OPTIMIZE_DEBUG is more
> > > inline with what it is and understandable than
> > > CONFIG_DEBUG_EXPERIENCE. The "OPTIMIZE" is the key word there.
> > > 
> > > -- Steve  
> > What about CONFIG_CC_OPTIMIZE_FOR_DEBUGGING? We alreay have
> > CONFIG_CC_OPTIMIZE_FOR_SIZE and CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE.
> 
> Yes I like that much better.
> 
> > 
> > And do we need to move it to existing configuration menu "General setup->
> > Compiler optimization level"? But I also want it appear in "kernel hacking"
> > since this is a debug option.
> 
> I understand why you would want it by debugging, but I think it does
> make more sense to be included with the above two other options, as
> they are all mutually exclusive.
> 
> This brings up the topic of creating config paradigms. That is, a way
> of saying "I want a debug kernel" and select one option that selects
> everything you would expect. Or perhaps we should have a:
> 
>  make debug_config
> 
Agree, I accomplish this by running script scripts/kconfig/merge_config.sh.

> that does it.
> 
> But that's a different topic. For now, I would just included it in
> init/Kconfig, and not worry about it not showing up in kernel hacking.
> 
> 
> -- Steve

-- 
Thanks,
Changbin Du

Re: [PATCH v2 4/5] kernel hacking: new config DEBUG_EXPERIENCE to apply GCC -Og optimization

[Du, Changbin]

On Thu, May 03, 2018 at 10:28:23AM -0400, Steven Rostedt wrote:
> On Thu, 3 May 2018 21:45:46 +0800
> "Du, Changbin"  wrote:
> 
> > > With that gcc comment, I still think CONFIG_OPTIMIZE_DEBUG is more
> > > inline with what it is and understandable than
> > > CONFIG_DEBUG_EXPERIENCE. The "OPTIMIZE" is the key word there.
> > > 
> > > -- Steve  
> > What about CONFIG_CC_OPTIMIZE_FOR_DEBUGGING? We alreay have
> > CONFIG_CC_OPTIMIZE_FOR_SIZE and CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE.
> 
> Yes I like that much better.
> 
> > 
> > And do we need to move it to existing configuration menu "General setup->
> > Compiler optimization level"? But I also want it appear in "kernel hacking"
> > since this is a debug option.
> 
> I understand why you would want it by debugging, but I think it does
> make more sense to be included with the above two other options, as
> they are all mutually exclusive.
> 
> This brings up the topic of creating config paradigms. That is, a way
> of saying "I want a debug kernel" and select one option that selects
> everything you would expect. Or perhaps we should have a:
> 
>  make debug_config
> 
Agree, I accomplish this by running script scripts/kconfig/merge_config.sh.

> that does it.
> 
> But that's a different topic. For now, I would just included it in
> init/Kconfig, and not worry about it not showing up in kernel hacking.
> 
> 
> -- Steve

-- 
Thanks,
Changbin Du

Re: [PATCH v2 0/5] kernel hacking: GCC optimization for debug experience (-Og)

[Du, Changbin]

On Wed, May 02, 2018 at 03:56:31PM +0100, Daniel Thompson wrote:
> On Wed, May 02, 2018 at 09:44:55PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du <changbin...@intel.com>
> > 
> > Hi all,
> > I know some kernel developers was searching for a method to dissable GCC
> > optimizations, probably they want to apply GCC '-O0' option. But since Linux
> > kernel replys on GCC optimization to remove some dead code, so '-O0' just
> > breaks the build. They do need this because they want to debug kernel with
> > qemu, simics, kgtp or kgdb.
> > 
> > Thanks for the GCC '-Og' optimization level introduced in GCC 4.8, which
> > offers a reasonable level of optimization while maintaining fast compilation
> > and a good debugging experience. It is similar to '-O1' while perfer keeping
> > debug ability over runtime speed. With '-Og', we can build a kernel with
> > better debug ability and little performance drop after some simple change.
> > 
> > In this series, firstly introduce a new config CONFIG_NO_AUTO_INLINE after 
> > two
> > fixes for this new option. With this option, only functions explicitly 
> > marked
> > with "inline" will  be inlined. This will allow the function tracer to trace
> > more functions because it only traces functions that the compiler has not
> > inlined.
> > 
> > Then introduce new config CONFIG_DEBUG_EXPERIENCE which apply '-Og'
> > optimization level for whole kernel, with a simple fix in fix_to_virt().
> > Currently this option is only tested on a QEMU gust and it works fine.
> > 
> > 
> > Comparison of vmlinux size: a bit smaller.
> > 
> > w/o CONFIG_DEBUG_EXPERIENCE
> > $ size vmlinux
> >textdata bss dec hex filename
> > 22665554   9709674  2920908 3529613621a9388 vmlinux
> > 
> > w/ CONFIG_DEBUG_EXPERIENCE
> > $ size vmlinux
> >textdata bss dec hex filename
> > 21499032   10102758 2920908 3452269820ec64a vmlinux
> > 
> > 
> > Comparison of system performance: a bit drop (~6%).
> > This benchmark of kernel compilation is suggested by Ingo Molnar.
> > https://lkml.org/lkml/2018/5/2/74
> 
> In my mind was the opposite question. When running on the same kernel
> does a kernel whose config contains CONFIG_DEBUG_EXPERIENCE build faster
> than one without (due to the disabled optimization passes).
> 
> To be honest this is more curiosity than a review comment though... if
> you have the figures please share, if not then don't sweat it on my
> account!
> 
> 
> Daniel.
>
Sorry I don't have the data yet. Per the comment in GCC, I think it should be a
little faster but not obviously.

> 
-- 
Thanks,
Changbin Du

Re: [PATCH v2 0/5] kernel hacking: GCC optimization for debug experience (-Og)

[Du, Changbin]

On Wed, May 02, 2018 at 03:56:31PM +0100, Daniel Thompson wrote:
> On Wed, May 02, 2018 at 09:44:55PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > Hi all,
> > I know some kernel developers was searching for a method to dissable GCC
> > optimizations, probably they want to apply GCC '-O0' option. But since Linux
> > kernel replys on GCC optimization to remove some dead code, so '-O0' just
> > breaks the build. They do need this because they want to debug kernel with
> > qemu, simics, kgtp or kgdb.
> > 
> > Thanks for the GCC '-Og' optimization level introduced in GCC 4.8, which
> > offers a reasonable level of optimization while maintaining fast compilation
> > and a good debugging experience. It is similar to '-O1' while perfer keeping
> > debug ability over runtime speed. With '-Og', we can build a kernel with
> > better debug ability and little performance drop after some simple change.
> > 
> > In this series, firstly introduce a new config CONFIG_NO_AUTO_INLINE after 
> > two
> > fixes for this new option. With this option, only functions explicitly 
> > marked
> > with "inline" will  be inlined. This will allow the function tracer to trace
> > more functions because it only traces functions that the compiler has not
> > inlined.
> > 
> > Then introduce new config CONFIG_DEBUG_EXPERIENCE which apply '-Og'
> > optimization level for whole kernel, with a simple fix in fix_to_virt().
> > Currently this option is only tested on a QEMU gust and it works fine.
> > 
> > 
> > Comparison of vmlinux size: a bit smaller.
> > 
> > w/o CONFIG_DEBUG_EXPERIENCE
> > $ size vmlinux
> >textdata bss dec hex filename
> > 22665554   9709674  2920908 3529613621a9388 vmlinux
> > 
> > w/ CONFIG_DEBUG_EXPERIENCE
> > $ size vmlinux
> >textdata bss dec hex filename
> > 21499032   10102758 2920908 3452269820ec64a vmlinux
> > 
> > 
> > Comparison of system performance: a bit drop (~6%).
> > This benchmark of kernel compilation is suggested by Ingo Molnar.
> > https://lkml.org/lkml/2018/5/2/74
> 
> In my mind was the opposite question. When running on the same kernel
> does a kernel whose config contains CONFIG_DEBUG_EXPERIENCE build faster
> than one without (due to the disabled optimization passes).
> 
> To be honest this is more curiosity than a review comment though... if
> you have the figures please share, if not then don't sweat it on my
> account!
> 
> 
> Daniel.
>
Sorry I don't have the data yet. Per the comment in GCC, I think it should be a
little faster but not obviously.

> 
-- 
Thanks,
Changbin Du

Re: [PATCH v2 4/5] kernel hacking: new config DEBUG_EXPERIENCE to apply GCC -Og optimization

[Du, Changbin]

On Wed, May 02, 2018 at 09:19:56PM -0400, Steven Rostedt wrote:
> On Wed, 2 May 2018 13:45:58 -0700
> Andrew Morton  wrote:
> 
> > On Wed, 2 May 2018 10:17:07 -0400 Steven Rostedt  
> > wrote:
> > 
> > > > Comparison of vmlinux size: a bit smaller.
> > > > 
> > > > w/o CONFIG_DEBUG_EXPERIENCE  
> > > 
> > > I hate the config name.
> > > 
> > > I probably can't come up with better ones but let's try:
> > > 
> > >  CONFIG_DEBUG_OPTIMIZE ?
> > >  CONFIG_OPTIMIZE_DEBUG ?
> > > 
> > > But "EXPERIENCE" sounds like I'm on some DEBUG LSD.  
> > 
> > Metoo, but the gcc people decided on "-Og: Optimize debugging
> > experience ..." and I think there are benefits if the kernel is to
> > align the naming with that.
> 
> I still see that as "Optimize debugging" and "experience" is just the
> platform of what was done.
> 
> With that gcc comment, I still think CONFIG_OPTIMIZE_DEBUG is more
> inline with what it is and understandable than
> CONFIG_DEBUG_EXPERIENCE. The "OPTIMIZE" is the key word there.
> 
> -- Steve
What about CONFIG_CC_OPTIMIZE_FOR_DEBUGGING? We alreay have
CONFIG_CC_OPTIMIZE_FOR_SIZE and CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE.

And do we need to move it to existing configuration menu "General setup->
Compiler optimization level"? But I also want it appear in "kernel hacking"
since this is a debug option.

-- 
Thanks,
Changbin Du

Re: [PATCH v2 4/5] kernel hacking: new config DEBUG_EXPERIENCE to apply GCC -Og optimization

[Du, Changbin]

On Wed, May 02, 2018 at 09:19:56PM -0400, Steven Rostedt wrote:
> On Wed, 2 May 2018 13:45:58 -0700
> Andrew Morton  wrote:
> 
> > On Wed, 2 May 2018 10:17:07 -0400 Steven Rostedt  
> > wrote:
> > 
> > > > Comparison of vmlinux size: a bit smaller.
> > > > 
> > > > w/o CONFIG_DEBUG_EXPERIENCE  
> > > 
> > > I hate the config name.
> > > 
> > > I probably can't come up with better ones but let's try:
> > > 
> > >  CONFIG_DEBUG_OPTIMIZE ?
> > >  CONFIG_OPTIMIZE_DEBUG ?
> > > 
> > > But "EXPERIENCE" sounds like I'm on some DEBUG LSD.  
> > 
> > Metoo, but the gcc people decided on "-Og: Optimize debugging
> > experience ..." and I think there are benefits if the kernel is to
> > align the naming with that.
> 
> I still see that as "Optimize debugging" and "experience" is just the
> platform of what was done.
> 
> With that gcc comment, I still think CONFIG_OPTIMIZE_DEBUG is more
> inline with what it is and understandable than
> CONFIG_DEBUG_EXPERIENCE. The "OPTIMIZE" is the key word there.
> 
> -- Steve
What about CONFIG_CC_OPTIMIZE_FOR_DEBUGGING? We alreay have
CONFIG_CC_OPTIMIZE_FOR_SIZE and CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE.

And do we need to move it to existing configuration menu "General setup->
Compiler optimization level"? But I also want it appear in "kernel hacking"
since this is a debug option.

-- 
Thanks,
Changbin Du

Re: [PATCH v2 5/5] asm-generic: fix build error in fix_to_virt with CONFIG_DEBUG_EXPERIENCE

[Du, Changbin]

On Wed, May 02, 2018 at 10:19:30AM -0400, Steven Rostedt wrote:
> On Wed,  2 May 2018 21:45:00 +0800
> changbin...@intel.com wrote:
> 
> > From: Changbin Du <changbin...@intel.com>
> > 
> > With '-Og' optimization level, GCC would not optimize a count for a loop
> > as a constant value. But BUILD_BUG_ON() only accept compile-time constant
> > values.
> > 
> > arch/arm/mm/mmu.o: In function `fix_to_virt':
> > /home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined 
> > reference to `__compiletime_assert_31'
> > Makefile:1051: recipe for target 'vmlinux' failed
> > make: *** [vmlinux] Error 1
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > ---
> >  include/asm-generic/fixmap.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
> > index 827e4d3..a6576d4 100644
> > --- a/include/asm-generic/fixmap.h
> > +++ b/include/asm-generic/fixmap.h
> > @@ -28,7 +28,8 @@
> >   */
> >  static __always_inline unsigned long fix_to_virt(const unsigned int idx)
> >  {
> > -   BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
> > +   BUILD_BUG_ON(__builtin_constant_p(idx) &&
> > +idx >= __end_of_fixed_addresses);
> 
> Hmm, this changes the check slightly. Perhaps we should only do this
> when your config is active:
> 
> {
>   BUILD_BUG_ON(
> /* CONFIG_DEBUG_OPTIMIZE may cause idx not to be constant */
> #ifdef CONFIG_DEBUG_OPTIMIZE
>   __builtin_constant_p(idx) &&
> #endif
>   idx >= __end_of_fixed_addresses);
> 
> }
I think fix_to_virt() is designed for constant idx only. So I think we should
fix it at the caller side by replacing it with __fix_to_virt().

--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
pte_t *pte;
struct map_desc map;

-   map.virtual = fix_to_virt(i);
+   map.virtual = __fix_to_virt(i);
pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), 
map.virtual);

> 
> -- Steve
> 
> > return __fix_to_virt(idx);
> >  }
> >  
> 

-- 
Thanks,
Changbin Du

Re: [PATCH v2 5/5] asm-generic: fix build error in fix_to_virt with CONFIG_DEBUG_EXPERIENCE

[Du, Changbin]

On Wed, May 02, 2018 at 10:19:30AM -0400, Steven Rostedt wrote:
> On Wed,  2 May 2018 21:45:00 +0800
> changbin...@intel.com wrote:
> 
> > From: Changbin Du 
> > 
> > With '-Og' optimization level, GCC would not optimize a count for a loop
> > as a constant value. But BUILD_BUG_ON() only accept compile-time constant
> > values.
> > 
> > arch/arm/mm/mmu.o: In function `fix_to_virt':
> > /home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined 
> > reference to `__compiletime_assert_31'
> > Makefile:1051: recipe for target 'vmlinux' failed
> > make: *** [vmlinux] Error 1
> > 
> > Signed-off-by: Changbin Du 
> > ---
> >  include/asm-generic/fixmap.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/asm-generic/fixmap.h b/include/asm-generic/fixmap.h
> > index 827e4d3..a6576d4 100644
> > --- a/include/asm-generic/fixmap.h
> > +++ b/include/asm-generic/fixmap.h
> > @@ -28,7 +28,8 @@
> >   */
> >  static __always_inline unsigned long fix_to_virt(const unsigned int idx)
> >  {
> > -   BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
> > +   BUILD_BUG_ON(__builtin_constant_p(idx) &&
> > +idx >= __end_of_fixed_addresses);
> 
> Hmm, this changes the check slightly. Perhaps we should only do this
> when your config is active:
> 
> {
>   BUILD_BUG_ON(
> /* CONFIG_DEBUG_OPTIMIZE may cause idx not to be constant */
> #ifdef CONFIG_DEBUG_OPTIMIZE
>   __builtin_constant_p(idx) &&
> #endif
>   idx >= __end_of_fixed_addresses);
> 
> }
I think fix_to_virt() is designed for constant idx only. So I think we should
fix it at the caller side by replacing it with __fix_to_virt().

--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
pte_t *pte;
struct map_desc map;

-   map.virtual = fix_to_virt(i);
+   map.virtual = __fix_to_virt(i);
pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), 
map.virtual);

> 
> -- Steve
> 
> > return __fix_to_virt(idx);
> >  }
> >  
> 

-- 
Thanks,
Changbin Du

Re: [PATCH v2 3/5] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du, Changbin]

On Wed, May 02, 2018 at 04:27:47PM -0400, Arnd Bergmann wrote:
> On Wed, May 2, 2018 at 9:44 AM,  <changbin...@intel.com> wrote:
> > From: Changbin Du <changbin...@intel.com>
> >
> > This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
> > this option will prevent the compiler from optimizing the kernel by
> > auto-inlining functions not marked with the inline keyword.
> >
> > With this option, only functions explicitly marked with "inline" will
> > be inlined. This will allow the function tracer to trace more functions
> > because it only traces functions that the compiler has not inlined.
> >
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> 
> Should this be closer to CONFIG_OPTIMIZE_INLINING or
> possibly mutually exclusive with it?
>
They are not related I think. CONFIG_OPTIMIZE_INLINING only has effect on
functions which are explicitly marked as inline.

>Arnd

-- 
Thanks,
Changbin Du

Re: [PATCH v2 3/5] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations

[Du, Changbin]

On Wed, May 02, 2018 at 04:27:47PM -0400, Arnd Bergmann wrote:
> On Wed, May 2, 2018 at 9:44 AM,   wrote:
> > From: Changbin Du 
> >
> > This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
> > this option will prevent the compiler from optimizing the kernel by
> > auto-inlining functions not marked with the inline keyword.
> >
> > With this option, only functions explicitly marked with "inline" will
> > be inlined. This will allow the function tracer to trace more functions
> > because it only traces functions that the compiler has not inlined.
> >
> > Signed-off-by: Changbin Du 
> > Cc: Steven Rostedt 
> 
> Should this be closer to CONFIG_OPTIMIZE_INLINING or
> possibly mutually exclusive with it?
>
They are not related I think. CONFIG_OPTIMIZE_INLINING only has effect on
functions which are explicitly marked as inline.

>Arnd

-- 
Thanks,
Changbin Du

Re: [PATCH 3/5] kernel hacking: new config NO_AUTO_INLINE to disable compiler atuo-inline optimizations

[Du, Changbin]

On Tue, May 01, 2018 at 10:54:20AM -0400, Steven Rostedt wrote:
> On Tue,  1 May 2018 21:00:12 +0800
> changbin...@intel.com wrote:
> 
> > From: Changbin Du <changbin...@intel.com>
> > 
> > This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
> > this option will make compiler not auto-inline kernel functions. By
> > enabling this option, all the kernel functions (including static ones)
> > will not be optimized out except those marked as inline or always_inline.
> > This is useful when you are using ftrace to understand the control flow
> > of kernel code or tracing some static functions.
> 
> I'm not against this patch, but it's up to others if this gets included
> or not.
> 
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> > ---
> >  Makefile  |  6 ++
> >  lib/Kconfig.debug | 13 +
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/Makefile b/Makefile
> > index 619a85a..eb694f6 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -775,6 +775,12 @@ KBUILD_CFLAGS  += $(call cc-option, 
> > -femit-struct-debug-baseonly) \
> >$(call cc-option,-fno-var-tracking)
> >  endif
> >  
> > +ifdef CONFIG_NO_AUTO_INLINE
> > +KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
> > +  $(call cc-option, -fno-inline-small-functions) \
> > +  $(call cc-option, -fno-inline-functions-called-once)
> > +endif
> > +
> >  ifdef CONFIG_FUNCTION_TRACER
> >  ifndef CC_FLAGS_FTRACE
> >  CC_FLAGS_FTRACE := -pg
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index c40c7b7..90f35ad 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -198,6 +198,19 @@ config GDB_SCRIPTS
> >   instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
> >   for further details.
> >  
> > +config NO_AUTO_INLINE
> > +   bool "Disable compiler atuo-inline optimizations"
> 
> typo: s/atuo/auto/
> 
> > +   default n
> > +   help
> > + This will make compiler not auto-inline kernel functions for
> > + optimization. By enabling this option, all the kernel functions
> > + (including static ones) will not be optimized out except those
> > + marked as inline or always_inline. This is useful when you are
> > + using ftrace to understand the control flow of kernel code or
> > + tracing some static functions.
> 
> Some grammar updates:
> 
> This will prevent the compiler from optimizing the kernel by
> auto-inlining functions not marked with the inline keyword.
> With this option, only functions explicitly marked with
> "inline" will be inlined. This will allow the function tracer
> to trace more functions because it only traces functions that
> the compiler has not inlined.
> 
> Enabling this function can help debugging a kernel if using
> the function tracer. But it can also change how the kernel
> works, because inlining functions may change the timing,
> which could make it difficult while debugging race conditions.
> 

Thanks for your kind grammar updates. I will update them. :)

> > +
> > + Use only if you want to debug the kernel.
> 
> The proper way to say the above is:
> 
> If unsure, select N
>
Agree.

> -- Steve
> 
> > +
> >  config ENABLE_WARN_DEPRECATED
> > bool "Enable __deprecated logic"
> > default y
> 


-- 
Thanks,
Changbin Du

Re: [PATCH 3/5] kernel hacking: new config NO_AUTO_INLINE to disable compiler atuo-inline optimizations

[Du, Changbin]

On Tue, May 01, 2018 at 10:54:20AM -0400, Steven Rostedt wrote:
> On Tue,  1 May 2018 21:00:12 +0800
> changbin...@intel.com wrote:
> 
> > From: Changbin Du 
> > 
> > This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
> > this option will make compiler not auto-inline kernel functions. By
> > enabling this option, all the kernel functions (including static ones)
> > will not be optimized out except those marked as inline or always_inline.
> > This is useful when you are using ftrace to understand the control flow
> > of kernel code or tracing some static functions.
> 
> I'm not against this patch, but it's up to others if this gets included
> or not.
> 
> > 
> > Signed-off-by: Changbin Du 
> > Cc: Steven Rostedt 
> > ---
> >  Makefile  |  6 ++
> >  lib/Kconfig.debug | 13 +
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/Makefile b/Makefile
> > index 619a85a..eb694f6 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -775,6 +775,12 @@ KBUILD_CFLAGS  += $(call cc-option, 
> > -femit-struct-debug-baseonly) \
> >$(call cc-option,-fno-var-tracking)
> >  endif
> >  
> > +ifdef CONFIG_NO_AUTO_INLINE
> > +KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
> > +  $(call cc-option, -fno-inline-small-functions) \
> > +  $(call cc-option, -fno-inline-functions-called-once)
> > +endif
> > +
> >  ifdef CONFIG_FUNCTION_TRACER
> >  ifndef CC_FLAGS_FTRACE
> >  CC_FLAGS_FTRACE := -pg
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index c40c7b7..90f35ad 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -198,6 +198,19 @@ config GDB_SCRIPTS
> >   instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
> >   for further details.
> >  
> > +config NO_AUTO_INLINE
> > +   bool "Disable compiler atuo-inline optimizations"
> 
> typo: s/atuo/auto/
> 
> > +   default n
> > +   help
> > + This will make compiler not auto-inline kernel functions for
> > + optimization. By enabling this option, all the kernel functions
> > + (including static ones) will not be optimized out except those
> > + marked as inline or always_inline. This is useful when you are
> > + using ftrace to understand the control flow of kernel code or
> > + tracing some static functions.
> 
> Some grammar updates:
> 
> This will prevent the compiler from optimizing the kernel by
> auto-inlining functions not marked with the inline keyword.
> With this option, only functions explicitly marked with
> "inline" will be inlined. This will allow the function tracer
> to trace more functions because it only traces functions that
> the compiler has not inlined.
> 
> Enabling this function can help debugging a kernel if using
> the function tracer. But it can also change how the kernel
> works, because inlining functions may change the timing,
> which could make it difficult while debugging race conditions.
> 

Thanks for your kind grammar updates. I will update them. :)

> > +
> > + Use only if you want to debug the kernel.
> 
> The proper way to say the above is:
> 
> If unsure, select N
>
Agree.

> -- Steve
> 
> > +
> >  config ENABLE_WARN_DEPRECATED
> > bool "Enable __deprecated logic"
> > default y
> 


-- 
Thanks,
Changbin Du

Re: [PATCH 4/5] kernel hacking: new config DEBUG_EXPERIENCE to apply GCC -Og optimization

[Du, Changbin]

On Tue, May 01, 2018 at 08:25:27AM -0700, Randy Dunlap wrote:
> Good morning.
> 
> On 05/01/2018 06:00 AM, changbin...@intel.com wrote:
> > From: Changbin Du <changbin...@intel.com>
> > 
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > ---
> >  Makefile |  4 
> >  include/linux/compiler-gcc.h |  2 +-
> >  include/linux/compiler.h |  2 +-
> >  lib/Kconfig.debug| 21 +
> >  4 files changed, 27 insertions(+), 2 deletions(-)
> > 
> 
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 90f35ad..2432e77d 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -211,6 +211,27 @@ config NO_AUTO_INLINE
> >  
> >   Use only if you want to debug the kernel.
> >  
> > +config DEBUG_EXPERIENCE
> > +   bool "Optimize for better debugging experience (-Og)"
> > +   default n
> > +   select NO_AUTO_INLINE
> > +   depends on !CC_OPTIMIZE_FOR_SIZE
> > +   help
> > + This will apply GCC '-Og' optimization level get supported from
> 
>  which is supported since
> 
> > + GCC 4.8. This optimization level offers a reasonable level of
> > + optimization while maintaining fast compilation and a good
> > + debugging experience. It is similar to '-O1' while perfer keeping
> 
>  while preferring to keep
> 
> > + debug ability over runtime speed. The overall performance will
> > + drop a bit.
> > +
> > + If enabling this option break your kernel, you should either
> 
> breaks
> 
> > + disable this or find a fix (mostly in the arch code). Currently
> > + this option has only be tested in qemu x86_64 guest.
> > +
> > + Use only if you want to debug the kernel, especially if you want
> > + to have better kernel debugging experience with gdb facilities
> > + like kgdb and qemu.
> > +
> >  config ENABLE_WARN_DEPRECATED
> > bool "Enable __deprecated logic"
> > default y
> > 
> 
> thanks,
> -- 
> ~Randy

Thanks for your correction, I will update.

-- 
Thanks,
Changbin Du

Re: [PATCH 4/5] kernel hacking: new config DEBUG_EXPERIENCE to apply GCC -Og optimization

[Du, Changbin]

On Tue, May 01, 2018 at 08:25:27AM -0700, Randy Dunlap wrote:
> Good morning.
> 
> On 05/01/2018 06:00 AM, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > 
> > Signed-off-by: Changbin Du 
> > ---
> >  Makefile |  4 
> >  include/linux/compiler-gcc.h |  2 +-
> >  include/linux/compiler.h |  2 +-
> >  lib/Kconfig.debug| 21 +
> >  4 files changed, 27 insertions(+), 2 deletions(-)
> > 
> 
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 90f35ad..2432e77d 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -211,6 +211,27 @@ config NO_AUTO_INLINE
> >  
> >   Use only if you want to debug the kernel.
> >  
> > +config DEBUG_EXPERIENCE
> > +   bool "Optimize for better debugging experience (-Og)"
> > +   default n
> > +   select NO_AUTO_INLINE
> > +   depends on !CC_OPTIMIZE_FOR_SIZE
> > +   help
> > + This will apply GCC '-Og' optimization level get supported from
> 
>  which is supported since
> 
> > + GCC 4.8. This optimization level offers a reasonable level of
> > + optimization while maintaining fast compilation and a good
> > + debugging experience. It is similar to '-O1' while perfer keeping
> 
>  while preferring to keep
> 
> > + debug ability over runtime speed. The overall performance will
> > + drop a bit.
> > +
> > + If enabling this option break your kernel, you should either
> 
> breaks
> 
> > + disable this or find a fix (mostly in the arch code). Currently
> > + this option has only be tested in qemu x86_64 guest.
> > +
> > + Use only if you want to debug the kernel, especially if you want
> > + to have better kernel debugging experience with gdb facilities
> > + like kgdb and qemu.
> > +
> >  config ENABLE_WARN_DEPRECATED
> > bool "Enable __deprecated logic"
> > default y
> > 
> 
> thanks,
> -- 
> ~Randy

Thanks for your correction, I will update.

-- 
Thanks,
Changbin Du

Re: [PATCH 2/5] regulator: add dummy of_find_regulator_by_node

[Du, Changbin]

On Wed, May 02, 2018 at 05:40:36AM +0900, Mark Brown wrote:
> On Tue, May 01, 2018 at 09:00:11PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du <changbin...@intel.com>
> > 
> > If device tree is not enabled, of_find_regulator_by_node() should have
> > a dummy function since the function call is still there.
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> 
> This appears to have no obvious connection with the cover letter for the
> series...  The first question here is if this is something best fixed
> with a stub or by fixing the users - is the lack of a stub pointing out
> some bugs in them?  I'm a bit worried about how we've been managing to
> avoid any build test issues here though, surely the various builders
> would have spotted a problem?

This is to fix build error after NO_AUTO_INLINE is introduced. If this option
is enabled, GCC will not auto-inline functions that are not explicitly marked
as inline.

In this case (no CONFIG_OF), the copmiler will report error in 
regulator_dev_lookup().
W/o NO_AUTO_INLINE, function of_get_regulator() is auto-inlined and then the 
call
to of_find_regulator_by_node() is optimized out since of_get_regulator() always
return NULL. W/ NO_AUTO_INLINE, the return value of of_get_regulator() is a 
variable
so the call to of_find_regulator_by_node() cannot be optimized out.

static struct regulator_dev *regulator_dev_lookup(struct device *dev,
  const char *supply)
{
struct regulator_dev *r = NULL;
struct device_node *node;
struct regulator_map *map;
const char *devname = NULL;

regulator_supply_alias(, );

/* first do a dt based lookup */
if (dev && dev->of_node) {
node = of_get_regulator(dev, supply);
if (node) {
r = of_find_regulator_by_node(node);
if (r)
return r;


It is safe we just provide a stub of_find_regulator_by_node() if no CONFIG_OF.

-- 
Thanks,
Changbin Du

Re: [PATCH 2/5] regulator: add dummy of_find_regulator_by_node

[Du, Changbin]

On Wed, May 02, 2018 at 05:40:36AM +0900, Mark Brown wrote:
> On Tue, May 01, 2018 at 09:00:11PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > If device tree is not enabled, of_find_regulator_by_node() should have
> > a dummy function since the function call is still there.
> > 
> > Signed-off-by: Changbin Du 
> 
> This appears to have no obvious connection with the cover letter for the
> series...  The first question here is if this is something best fixed
> with a stub or by fixing the users - is the lack of a stub pointing out
> some bugs in them?  I'm a bit worried about how we've been managing to
> avoid any build test issues here though, surely the various builders
> would have spotted a problem?

This is to fix build error after NO_AUTO_INLINE is introduced. If this option
is enabled, GCC will not auto-inline functions that are not explicitly marked
as inline.

In this case (no CONFIG_OF), the copmiler will report error in 
regulator_dev_lookup().
W/o NO_AUTO_INLINE, function of_get_regulator() is auto-inlined and then the 
call
to of_find_regulator_by_node() is optimized out since of_get_regulator() always
return NULL. W/ NO_AUTO_INLINE, the return value of of_get_regulator() is a 
variable
so the call to of_find_regulator_by_node() cannot be optimized out.

static struct regulator_dev *regulator_dev_lookup(struct device *dev,
  const char *supply)
{
struct regulator_dev *r = NULL;
struct device_node *node;
struct regulator_map *map;
const char *devname = NULL;

regulator_supply_alias(, );

/* first do a dt based lookup */
if (dev && dev->of_node) {
node = of_get_regulator(dev, supply);
if (node) {
r = of_find_regulator_by_node(node);
if (r)
return r;


It is safe we just provide a stub of_find_regulator_by_node() if no CONFIG_OF.

-- 
Thanks,
Changbin Du

Re: [PATCH 0/5] kernel hacking: GCC optimization for debug experience (-Og)

[Du, Changbin]

On Wed, May 02, 2018 at 09:33:15AM +0200, Ingo Molnar wrote:
> 
> * changbin...@intel.com  wrote:
> 
> > Comparison of system performance: a bit drop.
> > 
> > w/o CONFIG_DEBUG_EXPERIENCE
> > $ time make -j4
> > real6m43.619s
> > user19m5.160s
> > sys 2m20.287s
> > 
> > w/ CONFIG_DEBUG_EXPERIENCE
> > $ time make -j4
> > real6m55.054s
> > user19m11.129s
> > sys 2m36.345s
> 
> Sorry, that's not a proper kbuild performance measurement - there's no noise 
> estimation at all.
> 
> Below is a description that should produce more reliable numbers.
> 
> Thanks,
> 
>   Ingo
>
Thanks for your suggestion, I will try your tips to eliminate noise. Since it is
tested in KVM guest, so I just reboot the guest before testing. But in host side
I still need to consider these noises.

> 
> =>
> 
> So here's a pretty reliable way to measure kernel build time, which tries to 
> avoid 
> the various pitfalls of caching.
> 
> First I make sure that cpufreq is set to 'performance':
> 
>   for ((cpu=0; cpu<120; cpu++)); do
> G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
> [ -f $G ] && echo performance > $G
>   done
> 
> [ ... because it can be *really* annoying to discover that an ostensible 
>   performance regression was a cpufreq artifact ... again. ;-) ]
> 
> Then I copy a kernel tree to /tmp (ramfs) as root:
> 
>   cd /tmp
>   rm -rf linux
>   git clone ~/linux linux
>   cd linux
>   make defconfig >/dev/null
>   
> ... and then we can build the kernel in such a loop (as root again):
> 
>   perf stat --repeat 10 --null --pre  '\
>   cp -a kernel ../kernel.copy.$(date +%s); \
>   rm -rf *;\
>   git checkout .;  \
>   echo 1 > /proc/sys/vm/drop_caches;   \
>   find ../kernel* -type f | xargs cat >/dev/null;  \
>   make -j kernel >/dev/null;   \
>   make clean >/dev/null 2>&1;  \
>   sync'\
>\
>   make -j16 >/dev/null
> 
> ( I have tested these by pasting them into a terminal. Adjust the ~/linux 
> source 
>   git tree and the '-j16' to your system. )
> 
> Notes:
> 
>  - the 'pre' script portion is not timed by 'perf stat', only the raw build 
> times
> 
>  - we flush all caches via drop_caches and re-establish everything again, but:
> 
>  - we also introduce an intentional memory leak by slowly filling up ramfs 
> with 
>copies of 'kernel/', thus continously changing the layout of free memory, 
>cached data such as compiler binaries and the source code hierarchy. (Note 
>that the leak is about 8MB per iteration, so it isn't massive.)
> 
> With 10 iterations this is the statistical stability I get this on a big box:
> 
>  Performance counter stats for 'make -j128 kernel' (10 runs):
> 
>   26.346436425 seconds time elapsed(+- 0.19%)
> 
> ... which, despite a high iteration count of 10, is still surprisingly noisy, 
> right?
> 
> A 0.2% stddev is probably not enough to call a 0.7% regression with good 
> confidence, so I had to use *30* iterations to make measurement noise to be 
> about 
> an order of magnitude lower than the effect I'm trying to measure:
> 
>  Performance counter stats for 'make -j128' (30 runs):
> 
>   26.334767571 seconds time elapsed(+- 0.09% )
> 
> i.e. "26.334 +- 0.023" seconds is a number we can have pretty high confidence 
> in, 
> on this system.
> 
> And just to demonstrate that it's all real, I repeated the whole 30-iteration 
> measurement again:
> 
>  Performance counter stats for 'make -j128' (30 runs):
> 
>   26.311166142 seconds time elapsed(+- 0.07%)
> 

-- 
Thanks,
Changbin Du

Re: [PATCH 0/5] kernel hacking: GCC optimization for debug experience (-Og)

[Du, Changbin]

On Wed, May 02, 2018 at 09:33:15AM +0200, Ingo Molnar wrote:
> 
> * changbin...@intel.com  wrote:
> 
> > Comparison of system performance: a bit drop.
> > 
> > w/o CONFIG_DEBUG_EXPERIENCE
> > $ time make -j4
> > real6m43.619s
> > user19m5.160s
> > sys 2m20.287s
> > 
> > w/ CONFIG_DEBUG_EXPERIENCE
> > $ time make -j4
> > real6m55.054s
> > user19m11.129s
> > sys 2m36.345s
> 
> Sorry, that's not a proper kbuild performance measurement - there's no noise 
> estimation at all.
> 
> Below is a description that should produce more reliable numbers.
> 
> Thanks,
> 
>   Ingo
>
Thanks for your suggestion, I will try your tips to eliminate noise. Since it is
tested in KVM guest, so I just reboot the guest before testing. But in host side
I still need to consider these noises.

> 
> =>
> 
> So here's a pretty reliable way to measure kernel build time, which tries to 
> avoid 
> the various pitfalls of caching.
> 
> First I make sure that cpufreq is set to 'performance':
> 
>   for ((cpu=0; cpu<120; cpu++)); do
> G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
> [ -f $G ] && echo performance > $G
>   done
> 
> [ ... because it can be *really* annoying to discover that an ostensible 
>   performance regression was a cpufreq artifact ... again. ;-) ]
> 
> Then I copy a kernel tree to /tmp (ramfs) as root:
> 
>   cd /tmp
>   rm -rf linux
>   git clone ~/linux linux
>   cd linux
>   make defconfig >/dev/null
>   
> ... and then we can build the kernel in such a loop (as root again):
> 
>   perf stat --repeat 10 --null --pre  '\
>   cp -a kernel ../kernel.copy.$(date +%s); \
>   rm -rf *;\
>   git checkout .;  \
>   echo 1 > /proc/sys/vm/drop_caches;   \
>   find ../kernel* -type f | xargs cat >/dev/null;  \
>   make -j kernel >/dev/null;   \
>   make clean >/dev/null 2>&1;  \
>   sync'\
>\
>   make -j16 >/dev/null
> 
> ( I have tested these by pasting them into a terminal. Adjust the ~/linux 
> source 
>   git tree and the '-j16' to your system. )
> 
> Notes:
> 
>  - the 'pre' script portion is not timed by 'perf stat', only the raw build 
> times
> 
>  - we flush all caches via drop_caches and re-establish everything again, but:
> 
>  - we also introduce an intentional memory leak by slowly filling up ramfs 
> with 
>copies of 'kernel/', thus continously changing the layout of free memory, 
>cached data such as compiler binaries and the source code hierarchy. (Note 
>that the leak is about 8MB per iteration, so it isn't massive.)
> 
> With 10 iterations this is the statistical stability I get this on a big box:
> 
>  Performance counter stats for 'make -j128 kernel' (10 runs):
> 
>   26.346436425 seconds time elapsed(+- 0.19%)
> 
> ... which, despite a high iteration count of 10, is still surprisingly noisy, 
> right?
> 
> A 0.2% stddev is probably not enough to call a 0.7% regression with good 
> confidence, so I had to use *30* iterations to make measurement noise to be 
> about 
> an order of magnitude lower than the effect I'm trying to measure:
> 
>  Performance counter stats for 'make -j128' (30 runs):
> 
>   26.334767571 seconds time elapsed(+- 0.09% )
> 
> i.e. "26.334 +- 0.023" seconds is a number we can have pretty high confidence 
> in, 
> on this system.
> 
> And just to demonstrate that it's all real, I repeated the whole 30-iteration 
> measurement again:
> 
>  Performance counter stats for 'make -j128' (30 runs):
> 
>   26.311166142 seconds time elapsed(+- 0.07%)
> 

-- 
Thanks,
Changbin Du

Re: [PATCH] iommu/vt-d: fix shift-out-of-bounds in bug checking

[Du, Changbin]

Hello, any reviewer? Thanks!

On Fri, Apr 20, 2018 at 01:29:55PM +0800, changbin...@intel.com wrote:
> From: Changbin Du <changbin...@intel.com>
> 
> It allows to flush more than 4GB of device TLBs. So the mask should be
> 64bit wide. UBSAN captured this fault as below.
> 
> [3.760024] 
> 
> [3.768440] UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1348:3
> [3.774864] shift exponent 64 is too large for 32-bit type 'int'
> [3.780853] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G U
> 4.17.0-rc1+ #89
> [3.788661] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.2.8 
> 01/26/2016
> [3.796034] Call Trace:
> [3.798472]  
> [3.800479]  dump_stack+0x90/0xfb
> [3.803787]  ubsan_epilogue+0x9/0x40
> [3.807353]  __ubsan_handle_shift_out_of_bounds+0x10e/0x170
> [3.812916]  ? qi_flush_dev_iotlb+0x124/0x180
> [3.817261]  qi_flush_dev_iotlb+0x124/0x180
> [3.821437]  iommu_flush_dev_iotlb+0x94/0xf0
> [3.825698]  iommu_flush_iova+0x10b/0x1c0
> [3.829699]  ? fq_ring_free+0x1d0/0x1d0
> [3.833527]  iova_domain_flush+0x25/0x40
> [3.837448]  fq_flush_timeout+0x55/0x160
> [3.841368]  ? fq_ring_free+0x1d0/0x1d0
> [3.845200]  ? fq_ring_free+0x1d0/0x1d0
> [3.849034]  call_timer_fn+0xbe/0x310
> [3.852696]  ? fq_ring_free+0x1d0/0x1d0
> [3.856530]  run_timer_softirq+0x223/0x6e0
> [3.860625]  ? sched_clock+0x5/0x10
> [3.864108]  ? sched_clock+0x5/0x10
> [3.867594]  __do_softirq+0x1b5/0x6f5
> [3.871250]  irq_exit+0xd4/0x130
> [3.874470]  smp_apic_timer_interrupt+0xb8/0x2f0
> [3.879075]  apic_timer_interrupt+0xf/0x20
> [3.883159]  
> [3.885255] RIP: 0010:poll_idle+0x60/0xe7
> [3.889252] RSP: 0018:b1b201943e30 EFLAGS: 0246 ORIG_RAX: 
> ff13
> [3.896802] RAX: 8020 RBX: 008e RCX: 
> 001f
> [3.903918] RDX:  RSI: 2819aa06 RDI: 
> 
> [3.911031] RBP: 9e93c6b33280 R08: 0010f717d567 R09: 
> 0010d205
> [3.918146] R10: b1b201943df8 R11: 0001 R12: 
> e01b169d
> [3.925260] R13:  R14: b12aa400 R15: 
> 
> [3.932382]  cpuidle_enter_state+0xb4/0x470
> [3.936558]  do_idle+0x222/0x310
> [3.939779]  cpu_startup_entry+0x78/0x90
> [3.943693]  start_secondary+0x205/0x2e0
> [3.947607]  secondary_startup_64+0xa5/0xb0
> [3.951783] 
> 
> 
> Signed-off-by: Changbin Du <changbin...@intel.com>
> ---
>  drivers/iommu/dmar.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index accf5838..e4ae600 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -1345,7 +1345,7 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 
> sid, u16 qdep,
>   struct qi_desc desc;
>  
>   if (mask) {
> - BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1));
> + BUG_ON(addr & ((1ULL << (VTD_PAGE_SHIFT + mask)) - 1));
>   addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
>   desc.high = QI_DEV_IOTLB_ADDR(addr) | QI_DEV_IOTLB_SIZE;
>   } else
> -- 
> 2.7.4
> 

-- 
Thanks,
Changbin Du

Re: [PATCH] iommu/vt-d: fix shift-out-of-bounds in bug checking

[Du, Changbin]

Hello, any reviewer? Thanks!

On Fri, Apr 20, 2018 at 01:29:55PM +0800, changbin...@intel.com wrote:
> From: Changbin Du 
> 
> It allows to flush more than 4GB of device TLBs. So the mask should be
> 64bit wide. UBSAN captured this fault as below.
> 
> [3.760024] 
> 
> [3.768440] UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1348:3
> [3.774864] shift exponent 64 is too large for 32-bit type 'int'
> [3.780853] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G U
> 4.17.0-rc1+ #89
> [3.788661] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.2.8 
> 01/26/2016
> [3.796034] Call Trace:
> [3.798472]  
> [3.800479]  dump_stack+0x90/0xfb
> [3.803787]  ubsan_epilogue+0x9/0x40
> [3.807353]  __ubsan_handle_shift_out_of_bounds+0x10e/0x170
> [3.812916]  ? qi_flush_dev_iotlb+0x124/0x180
> [3.817261]  qi_flush_dev_iotlb+0x124/0x180
> [3.821437]  iommu_flush_dev_iotlb+0x94/0xf0
> [3.825698]  iommu_flush_iova+0x10b/0x1c0
> [3.829699]  ? fq_ring_free+0x1d0/0x1d0
> [3.833527]  iova_domain_flush+0x25/0x40
> [3.837448]  fq_flush_timeout+0x55/0x160
> [3.841368]  ? fq_ring_free+0x1d0/0x1d0
> [3.845200]  ? fq_ring_free+0x1d0/0x1d0
> [3.849034]  call_timer_fn+0xbe/0x310
> [3.852696]  ? fq_ring_free+0x1d0/0x1d0
> [3.856530]  run_timer_softirq+0x223/0x6e0
> [3.860625]  ? sched_clock+0x5/0x10
> [3.864108]  ? sched_clock+0x5/0x10
> [3.867594]  __do_softirq+0x1b5/0x6f5
> [3.871250]  irq_exit+0xd4/0x130
> [3.874470]  smp_apic_timer_interrupt+0xb8/0x2f0
> [3.879075]  apic_timer_interrupt+0xf/0x20
> [3.883159]  
> [3.885255] RIP: 0010:poll_idle+0x60/0xe7
> [3.889252] RSP: 0018:b1b201943e30 EFLAGS: 0246 ORIG_RAX: 
> ff13
> [3.896802] RAX: 8020 RBX: 008e RCX: 
> 001f
> [3.903918] RDX:  RSI: 2819aa06 RDI: 
> 
> [3.911031] RBP: 9e93c6b33280 R08: 0010f717d567 R09: 
> 0010d205
> [3.918146] R10: b1b201943df8 R11: 0001 R12: 
> e01b169d
> [3.925260] R13:  R14: b12aa400 R15: 
> 
> [3.932382]  cpuidle_enter_state+0xb4/0x470
> [3.936558]  do_idle+0x222/0x310
> [3.939779]  cpu_startup_entry+0x78/0x90
> [3.943693]  start_secondary+0x205/0x2e0
> [3.947607]  secondary_startup_64+0xa5/0xb0
> [3.951783] 
> 
> 
> Signed-off-by: Changbin Du 
> ---
>  drivers/iommu/dmar.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index accf5838..e4ae600 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -1345,7 +1345,7 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 
> sid, u16 qdep,
>   struct qi_desc desc;
>  
>   if (mask) {
> - BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1));
> + BUG_ON(addr & ((1ULL << (VTD_PAGE_SHIFT + mask)) - 1));
>   addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
>   desc.high = QI_DEV_IOTLB_ADDR(addr) | QI_DEV_IOTLB_SIZE;
>   } else
> -- 
> 2.7.4
> 

-- 
Thanks,
Changbin Du

Re: [PATCH] Documentation: fix reST markup error in driver-api/usb/typec.rst

[Du, Changbin]

On Sun, Apr 08, 2018 at 09:19:58AM +0200, Greg KH wrote:
> On Sun, Apr 08, 2018 at 10:47:12AM +0800, changbin...@intel.com wrote:
> > From: Changbin Du <changbin...@intel.com>
> > 
> > There is an format error in driver-api/usb/typec.rst that breaks sphinx
> > docs building.
> > 
> > reST markup error:
> > /home/changbin/work/linux/Documentation/driver-api/usb/typec.rst:215: 
> > (SEVERE/4) Unexpected section title or transition.
> > 
> > 
> > Documentation/Makefile:68: recipe for target 'htmldocs' failed
> > make[1]: *** [htmldocs] Error 1
> > Makefile:1527: recipe for target 'htmldocs' failed
> > make: *** [htmldocs] Error 2
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > ---
> >  Documentation/driver-api/usb/typec.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thanks, someone else already sent this, sorry.  I'll be sending it
> onward after 4.17-rc1 is out.
>
No problem. Thanks for your quick checking!

> greg k-h

-- 
Thanks,
Changbin Du

Re: [PATCH] Documentation: fix reST markup error in driver-api/usb/typec.rst

[Du, Changbin]

On Sun, Apr 08, 2018 at 09:19:58AM +0200, Greg KH wrote:
> On Sun, Apr 08, 2018 at 10:47:12AM +0800, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > There is an format error in driver-api/usb/typec.rst that breaks sphinx
> > docs building.
> > 
> > reST markup error:
> > /home/changbin/work/linux/Documentation/driver-api/usb/typec.rst:215: 
> > (SEVERE/4) Unexpected section title or transition.
> > 
> > 
> > Documentation/Makefile:68: recipe for target 'htmldocs' failed
> > make[1]: *** [htmldocs] Error 1
> > Makefile:1527: recipe for target 'htmldocs' failed
> > make: *** [htmldocs] Error 2
> > 
> > Signed-off-by: Changbin Du 
> > ---
> >  Documentation/driver-api/usb/typec.rst | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thanks, someone else already sent this, sorry.  I'll be sending it
> onward after 4.17-rc1 is out.
>
No problem. Thanks for your quick checking!

> greg k-h

-- 
Thanks,
Changbin Du

Re: [PATCH] perf trace: remove redundant ')'

[Du, Changbin]

On Tue, Apr 03, 2018 at 04:19:07PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 28, 2018 at 03:26:31PM +0800, Du, Changbin escreveu:
> > Hi Arnaldo,
> > Just a kind reminder. Hope you didn't forget this.
> 
> Ok, applied.
> 
> - Arnaldo
>  
Got it, thanks!

Re: [PATCH] perf trace: remove redundant ')'

[Du, Changbin]

On Tue, Apr 03, 2018 at 04:19:07PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 28, 2018 at 03:26:31PM +0800, Du, Changbin escreveu:
> > Hi Arnaldo,
> > Just a kind reminder. Hope you didn't forget this.
> 
> Ok, applied.
> 
> - Arnaldo
>  
Got it, thanks!

Re: [PATCH] perf trace: remove redundant ')'

[Du, Changbin]

Hi Arnaldo,
Just a kind reminder. Hope you didn't forget this.

On Fri, Mar 16, 2018 at 09:50:45AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Mar 16, 2018 at 03:51:09PM +0800, Du, Changbin escreveu:
> > Hi Arnaldo, How about this simple one? Thanks.
> > 
> > On Tue, Mar 13, 2018 at 06:40:01PM +0800, changbin...@intel.com wrote:
> > > From: Changbin Du <changbin...@intel.com>
> > > 
> > > There is a redundant ')' at the tail of each event. So remove it.
> > > $ sudo perf trace --no-syscalls -e 'kmem:*' -a
> > >899.342 kmem:kfree:(vfs_writev+0xb9) call_site=9c453979 
> > > ptr=(nil))
> > >899.344 kmem:kfree:(___sys_recvmsg+0x188) call_site=9c9b8b88 
> > > ptr=(nil))
> > > 
> > > Signed-off-by: Changbin Du <changbin...@intel.com>
> > > ---
> > >  tools/perf/builtin-trace.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > > index e7f1b18..7273f5f 100644
> > > --- a/tools/perf/builtin-trace.c
> > > +++ b/tools/perf/builtin-trace.c
> > > @@ -1959,7 +1959,7 @@ static int trace__event_handler(struct trace 
> > > *trace, struct perf_evsel *evsel,
> > > trace->output);
> > >   }
> > >  
> > > - fprintf(trace->output, ")\n");
> > > + fprintf(trace->output, "\n");
> 
> It looks simple on the surface, but I couldn't quickly recall why this
> ')' was put there in the first place... So I left for later to do a 'git
> blame' on this file, etc.
> 
> - Arnaldo
> 
> > >   if (callchain_ret > 0)
> > >   trace__fprintf_callchain(trace, sample);
> > > -- 
> > > 2.7.4
> > > 
> > 
> > -- 
> > Thanks,
> > Changbin Du

-- 
Thanks,
Changbin Du

Re: [PATCH] perf trace: remove redundant ')'

[Du, Changbin]

Hi Arnaldo,
Just a kind reminder. Hope you didn't forget this.

On Fri, Mar 16, 2018 at 09:50:45AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Mar 16, 2018 at 03:51:09PM +0800, Du, Changbin escreveu:
> > Hi Arnaldo, How about this simple one? Thanks.
> > 
> > On Tue, Mar 13, 2018 at 06:40:01PM +0800, changbin...@intel.com wrote:
> > > From: Changbin Du 
> > > 
> > > There is a redundant ')' at the tail of each event. So remove it.
> > > $ sudo perf trace --no-syscalls -e 'kmem:*' -a
> > >899.342 kmem:kfree:(vfs_writev+0xb9) call_site=9c453979 
> > > ptr=(nil))
> > >899.344 kmem:kfree:(___sys_recvmsg+0x188) call_site=9c9b8b88 
> > > ptr=(nil))
> > > 
> > > Signed-off-by: Changbin Du 
> > > ---
> > >  tools/perf/builtin-trace.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > > index e7f1b18..7273f5f 100644
> > > --- a/tools/perf/builtin-trace.c
> > > +++ b/tools/perf/builtin-trace.c
> > > @@ -1959,7 +1959,7 @@ static int trace__event_handler(struct trace 
> > > *trace, struct perf_evsel *evsel,
> > > trace->output);
> > >   }
> > >  
> > > - fprintf(trace->output, ")\n");
> > > + fprintf(trace->output, "\n");
> 
> It looks simple on the surface, but I couldn't quickly recall why this
> ')' was put there in the first place... So I left for later to do a 'git
> blame' on this file, etc.
> 
> - Arnaldo
> 
> > >   if (callchain_ret > 0)
> > >   trace__fprintf_callchain(trace, sample);
> > > -- 
> > > 2.7.4
> > > 
> > 
> > -- 
> > Thanks,
> > Changbin Du

-- 
Thanks,
Changbin Du

Re: [PATCH v2 1/4] selftests/Makefile: append a slash to env variable OUTPUT

[Du, Changbin]

On Tue, Mar 27, 2018 at 03:19:26PM -0600, Shuah Khan wrote:
> On 03/26/2018 09:11 PM, changbin...@intel.com wrote:
> > From: Changbin Du <changbin...@intel.com>
> > 
> > The tools/build/Makefile.build use 'OUTPUT' variable as below example:
> > objprefix:= $(subst ./,,$(OUTPUT)$(dir)/)
> > 
> > So it requires the 'OUTPUT' already has a slash at the end.
> > 
> > This patch can kill below odd paths:
> > make[3]: Entering directory '/home/changbin/work/linux/tools/gpio'
> >   CC   /home/changbin/work/linux/tools/testing/selftests/gpiolsgpio.o
> >   CC   
> > /home/changbin/work/linux/tools/testing/selftests/gpiogpio-utils.o
> >   LD   /home/changbin/work/linux/tools/testing/selftests/gpiolsgpio-in.o
> > 
> > A correct path should be:
> > /home/changbin/work/linux/tools/testing/selftests/gpio/lsgpio.o
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> 
> Are you seeing this when you run "make kselftest" - if gpio is the
> only test compile that fails, it should be fixed in gpio/Makefile,
> not is the common Makefile.
>
I only saw error in gpio, but I also saw some kselftest Makefiles having string 
concatenation
as '$(OUTPUT)$(dir)'. So the rule is not aligned all over. They just didn't 
produce any errors
so far.

By the way, is there a basic test for kselftest infrastructure? It seems it was 
always
reporting error when building it :(

> thanks,
> -- Shuah

-- 
Thanks,
Changbin Du

Re: [PATCH v2 1/4] selftests/Makefile: append a slash to env variable OUTPUT

[Du, Changbin]

On Tue, Mar 27, 2018 at 03:19:26PM -0600, Shuah Khan wrote:
> On 03/26/2018 09:11 PM, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > The tools/build/Makefile.build use 'OUTPUT' variable as below example:
> > objprefix:= $(subst ./,,$(OUTPUT)$(dir)/)
> > 
> > So it requires the 'OUTPUT' already has a slash at the end.
> > 
> > This patch can kill below odd paths:
> > make[3]: Entering directory '/home/changbin/work/linux/tools/gpio'
> >   CC   /home/changbin/work/linux/tools/testing/selftests/gpiolsgpio.o
> >   CC   
> > /home/changbin/work/linux/tools/testing/selftests/gpiogpio-utils.o
> >   LD   /home/changbin/work/linux/tools/testing/selftests/gpiolsgpio-in.o
> > 
> > A correct path should be:
> > /home/changbin/work/linux/tools/testing/selftests/gpio/lsgpio.o
> > 
> > Signed-off-by: Changbin Du 
> 
> Are you seeing this when you run "make kselftest" - if gpio is the
> only test compile that fails, it should be fixed in gpio/Makefile,
> not is the common Makefile.
>
I only saw error in gpio, but I also saw some kselftest Makefiles having string 
concatenation
as '$(OUTPUT)$(dir)'. So the rule is not aligned all over. They just didn't 
produce any errors
so far.

By the way, is there a basic test for kselftest infrastructure? It seems it was 
always
reporting error when building it :(

> thanks,
> -- Shuah

-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

On Tue, Mar 27, 2018 at 11:52:27AM +0200, Daniel Borkmann wrote:
> On 03/27/2018 11:00 AM, Du, Changbin wrote:
> > On Tue, Mar 27, 2018 at 10:52:57AM +0200, Daniel Borkmann wrote:
> >> On 03/27/2018 05:06 AM, Du, Changbin wrote:
> >>> On Mon, Mar 26, 2018 at 08:02:30PM -0700, Alexei Starovoitov wrote:
> >>>> On Tue, Mar 27, 2018 at 10:20:10AM +0800, Du, Changbin wrote:
> >>>>> On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> >>>>>> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> >>>>>>> Signed-off-by: Changbin Du <changbin...@intel.com>
> >>>>>>> ---
> >>>>>>>  tools/testing/selftests/bpf/Makefile | 5 +++--
> >>>>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/tools/testing/selftests/bpf/Makefile 
> >>>>>>> b/tools/testing/selftests/bpf/Makefile
> >>>>>>> index 5c43c18..dc0fdc8 100644
> >>>>>>> --- a/tools/testing/selftests/bpf/Makefile
> >>>>>>> +++ b/tools/testing/selftests/bpf/Makefile
> >>>>>>> @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >>>>>>>GENFLAGS := -DHAVE_GENHDR
> >>>>>>>  endif
> >>>>>>>  
> >>>>>>> -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> >>>>>>> -I../../../include
> >>>>>>> +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> >>>>>>> +   -I../../../include -I../../../../usr/include
> >>>>>>>  LDLIBS += -lcap -lelf -lrt -lpthread
> >>>>>>>  
> >>>>>>>  # Order correspond to 'make run_tests' order
> >>>>>>> @@ -62,7 +63,7 @@ else
> >>>>>>>CPU ?= generic
> >>>>>>>  endif
> >>>>>>>  
> >>>>>>> -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> >>>>>>> +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> >>>>>>> -I../../../../usr/include \
> >>>>>>> -Wno-compare-distinct-pointer-types
> >>>>>>
> >>>>>> Nack.
> >>>>>> I suspect that will break the build for everyone else who's doing it 
> >>>>>> in the directory
> >>>>>> itself instead of the outer one.
> >>>>>
> >>>>> This one? But I didn't see any problem.
> >>>>
> >>>> because the build was lucky and additional path ../../../../usr/include 
> >>>> didn't point
> >>>> to a valid dir?
> >>
> >> Agree.
> >>
> >>> I am sorry but I don't understand why you mean *lucky*. Of cause, the 
> >>> path is valid.
> >>
> >> The problem is that this suddenly requires users to do a 'make 
> >> headers_install' in
> >> order to populate usr/include/ directory in the first place. While it's 
> >> annoying
> >> enough for BPF samples where this is needed, I absolutely don't want to 
> >> introduce
> >> this for BPF kselftests. It's the wrong approach. Besides, in tools infra, 
> >> there is
> >> a tools/arch/*/include/uapi/asm/bitsperlong.h header copy already, so we 
> >> really need
> >> to use that instead. Please adapt your patch accordingly and respin. 
> >> Please also Cc
> >> us and net...@vger.kernel.org for BPF kselftests changes.
> >>
> > Thanks for the explanation. So we expect that tools/arch/*/include is in 
> > the searching list, right?
> > The corrent makefile seems not. How do you get this built?
> 
> E.g. take a look at tools/include/asm/barrier.h or 
> tools/include/uapi/asm/bpf_perf_event.h
> just to name two examples. We'd need something similar to this which then 
> points to the
> arch specific includes.
> 
> Thanks,
> Daniel
>
ok, I see. But I am going to skip this fix this time. Because one get fixed, 
another appears.
IMHO, It doesn't sound like a good idea to sync all these files manually. We 
should have
better solution I think.

clang -I. -I./include/uapi -I../../../include/uapi 
-Wno-compare-distinct-pointer-types \
 -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - |  \
llc -march=bpf -mcpu=generic -filetype=obj -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
In file included from test_pkt_access.c:12:
/usr/include/linux/ip.h:20:10: fatal error: 'asm/byteorder.h' file not found
#include 

 
> > changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make -p
> > [...]
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
> > In file included from test_pkt_access.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> > #include 
> > 
> > 
> 

-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

On Tue, Mar 27, 2018 at 11:52:27AM +0200, Daniel Borkmann wrote:
> On 03/27/2018 11:00 AM, Du, Changbin wrote:
> > On Tue, Mar 27, 2018 at 10:52:57AM +0200, Daniel Borkmann wrote:
> >> On 03/27/2018 05:06 AM, Du, Changbin wrote:
> >>> On Mon, Mar 26, 2018 at 08:02:30PM -0700, Alexei Starovoitov wrote:
> >>>> On Tue, Mar 27, 2018 at 10:20:10AM +0800, Du, Changbin wrote:
> >>>>> On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> >>>>>> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> >>>>>>> Signed-off-by: Changbin Du 
> >>>>>>> ---
> >>>>>>>  tools/testing/selftests/bpf/Makefile | 5 +++--
> >>>>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/tools/testing/selftests/bpf/Makefile 
> >>>>>>> b/tools/testing/selftests/bpf/Makefile
> >>>>>>> index 5c43c18..dc0fdc8 100644
> >>>>>>> --- a/tools/testing/selftests/bpf/Makefile
> >>>>>>> +++ b/tools/testing/selftests/bpf/Makefile
> >>>>>>> @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >>>>>>>GENFLAGS := -DHAVE_GENHDR
> >>>>>>>  endif
> >>>>>>>  
> >>>>>>> -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> >>>>>>> -I../../../include
> >>>>>>> +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> >>>>>>> +   -I../../../include -I../../../../usr/include
> >>>>>>>  LDLIBS += -lcap -lelf -lrt -lpthread
> >>>>>>>  
> >>>>>>>  # Order correspond to 'make run_tests' order
> >>>>>>> @@ -62,7 +63,7 @@ else
> >>>>>>>CPU ?= generic
> >>>>>>>  endif
> >>>>>>>  
> >>>>>>> -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> >>>>>>> +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> >>>>>>> -I../../../../usr/include \
> >>>>>>> -Wno-compare-distinct-pointer-types
> >>>>>>
> >>>>>> Nack.
> >>>>>> I suspect that will break the build for everyone else who's doing it 
> >>>>>> in the directory
> >>>>>> itself instead of the outer one.
> >>>>>
> >>>>> This one? But I didn't see any problem.
> >>>>
> >>>> because the build was lucky and additional path ../../../../usr/include 
> >>>> didn't point
> >>>> to a valid dir?
> >>
> >> Agree.
> >>
> >>> I am sorry but I don't understand why you mean *lucky*. Of cause, the 
> >>> path is valid.
> >>
> >> The problem is that this suddenly requires users to do a 'make 
> >> headers_install' in
> >> order to populate usr/include/ directory in the first place. While it's 
> >> annoying
> >> enough for BPF samples where this is needed, I absolutely don't want to 
> >> introduce
> >> this for BPF kselftests. It's the wrong approach. Besides, in tools infra, 
> >> there is
> >> a tools/arch/*/include/uapi/asm/bitsperlong.h header copy already, so we 
> >> really need
> >> to use that instead. Please adapt your patch accordingly and respin. 
> >> Please also Cc
> >> us and net...@vger.kernel.org for BPF kselftests changes.
> >>
> > Thanks for the explanation. So we expect that tools/arch/*/include is in 
> > the searching list, right?
> > The corrent makefile seems not. How do you get this built?
> 
> E.g. take a look at tools/include/asm/barrier.h or 
> tools/include/uapi/asm/bpf_perf_event.h
> just to name two examples. We'd need something similar to this which then 
> points to the
> arch specific includes.
> 
> Thanks,
> Daniel
>
ok, I see. But I am going to skip this fix this time. Because one get fixed, 
another appears.
IMHO, It doesn't sound like a good idea to sync all these files manually. We 
should have
better solution I think.

clang -I. -I./include/uapi -I../../../include/uapi 
-Wno-compare-distinct-pointer-types \
 -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - |  \
llc -march=bpf -mcpu=generic -filetype=obj -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
In file included from test_pkt_access.c:12:
/usr/include/linux/ip.h:20:10: fatal error: 'asm/byteorder.h' file not found
#include 

 
> > changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make -p
> > [...]
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
> > In file included from test_pkt_access.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> > #include 
> > 
> > 
> 

-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

On Tue, Mar 27, 2018 at 10:52:57AM +0200, Daniel Borkmann wrote:
> On 03/27/2018 05:06 AM, Du, Changbin wrote:
> > On Mon, Mar 26, 2018 at 08:02:30PM -0700, Alexei Starovoitov wrote:
> >> On Tue, Mar 27, 2018 at 10:20:10AM +0800, Du, Changbin wrote:
> >>> On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> >>>> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> >>>>> Signed-off-by: Changbin Du <changbin...@intel.com>
> >>>>> ---
> >>>>>  tools/testing/selftests/bpf/Makefile | 5 +++--
> >>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/tools/testing/selftests/bpf/Makefile 
> >>>>> b/tools/testing/selftests/bpf/Makefile
> >>>>> index 5c43c18..dc0fdc8 100644
> >>>>> --- a/tools/testing/selftests/bpf/Makefile
> >>>>> +++ b/tools/testing/selftests/bpf/Makefile
> >>>>> @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >>>>>GENFLAGS := -DHAVE_GENHDR
> >>>>>  endif
> >>>>>  
> >>>>> -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> >>>>> -I../../../include
> >>>>> +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> >>>>> + -I../../../include -I../../../../usr/include
> >>>>>  LDLIBS += -lcap -lelf -lrt -lpthread
> >>>>>  
> >>>>>  # Order correspond to 'make run_tests' order
> >>>>> @@ -62,7 +63,7 @@ else
> >>>>>CPU ?= generic
> >>>>>  endif
> >>>>>  
> >>>>> -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> >>>>> +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> >>>>> -I../../../../usr/include \
> >>>>>   -Wno-compare-distinct-pointer-types
> >>>>
> >>>> Nack.
> >>>> I suspect that will break the build for everyone else who's doing it in 
> >>>> the directory
> >>>> itself instead of the outer one.
> >>>
> >>> This one? But I didn't see any problem.
> >>
> >> because the build was lucky and additional path ../../../../usr/include 
> >> didn't point
> >> to a valid dir?
> 
> Agree.
> 
> > I am sorry but I don't understand why you mean *lucky*. Of cause, the path 
> > is valid.
> 
> The problem is that this suddenly requires users to do a 'make 
> headers_install' in
> order to populate usr/include/ directory in the first place. While it's 
> annoying
> enough for BPF samples where this is needed, I absolutely don't want to 
> introduce
> this for BPF kselftests. It's the wrong approach. Besides, in tools infra, 
> there is
> a tools/arch/*/include/uapi/asm/bitsperlong.h header copy already, so we 
> really need
> to use that instead. Please adapt your patch accordingly and respin. Please 
> also Cc
> us and net...@vger.kernel.org for BPF kselftests changes.
> 
> Thanks,
> Daniel
Thanks for the explanation. So we expect that tools/arch/*/include is in the 
searching list, right?
The corrent makefile seems not. How do you get this built?

changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make -p
[...]
clang -I. -I./include/uapi -I../../../include/uapi 
-Wno-compare-distinct-pointer-types \
 -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - |  \
llc -march=bpf -mcpu=generic -filetype=obj -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
In file included from test_pkt_access.c:9:
In file included from ../../../include/uapi/linux/bpf.h:11:
In file included from ./include/uapi/linux/types.h:5:
/usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
file not found
#include 


-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

On Tue, Mar 27, 2018 at 10:52:57AM +0200, Daniel Borkmann wrote:
> On 03/27/2018 05:06 AM, Du, Changbin wrote:
> > On Mon, Mar 26, 2018 at 08:02:30PM -0700, Alexei Starovoitov wrote:
> >> On Tue, Mar 27, 2018 at 10:20:10AM +0800, Du, Changbin wrote:
> >>> On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> >>>> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> >>>>> Signed-off-by: Changbin Du 
> >>>>> ---
> >>>>>  tools/testing/selftests/bpf/Makefile | 5 +++--
> >>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/tools/testing/selftests/bpf/Makefile 
> >>>>> b/tools/testing/selftests/bpf/Makefile
> >>>>> index 5c43c18..dc0fdc8 100644
> >>>>> --- a/tools/testing/selftests/bpf/Makefile
> >>>>> +++ b/tools/testing/selftests/bpf/Makefile
> >>>>> @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >>>>>GENFLAGS := -DHAVE_GENHDR
> >>>>>  endif
> >>>>>  
> >>>>> -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> >>>>> -I../../../include
> >>>>> +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> >>>>> + -I../../../include -I../../../../usr/include
> >>>>>  LDLIBS += -lcap -lelf -lrt -lpthread
> >>>>>  
> >>>>>  # Order correspond to 'make run_tests' order
> >>>>> @@ -62,7 +63,7 @@ else
> >>>>>CPU ?= generic
> >>>>>  endif
> >>>>>  
> >>>>> -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> >>>>> +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> >>>>> -I../../../../usr/include \
> >>>>>   -Wno-compare-distinct-pointer-types
> >>>>
> >>>> Nack.
> >>>> I suspect that will break the build for everyone else who's doing it in 
> >>>> the directory
> >>>> itself instead of the outer one.
> >>>
> >>> This one? But I didn't see any problem.
> >>
> >> because the build was lucky and additional path ../../../../usr/include 
> >> didn't point
> >> to a valid dir?
> 
> Agree.
> 
> > I am sorry but I don't understand why you mean *lucky*. Of cause, the path 
> > is valid.
> 
> The problem is that this suddenly requires users to do a 'make 
> headers_install' in
> order to populate usr/include/ directory in the first place. While it's 
> annoying
> enough for BPF samples where this is needed, I absolutely don't want to 
> introduce
> this for BPF kselftests. It's the wrong approach. Besides, in tools infra, 
> there is
> a tools/arch/*/include/uapi/asm/bitsperlong.h header copy already, so we 
> really need
> to use that instead. Please adapt your patch accordingly and respin. Please 
> also Cc
> us and net...@vger.kernel.org for BPF kselftests changes.
> 
> Thanks,
> Daniel
Thanks for the explanation. So we expect that tools/arch/*/include is in the 
searching list, right?
The corrent makefile seems not. How do you get this built?

changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make -p
[...]
clang -I. -I./include/uapi -I../../../include/uapi 
-Wno-compare-distinct-pointer-types \
 -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - |  \
llc -march=bpf -mcpu=generic -filetype=obj -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
In file included from test_pkt_access.c:9:
In file included from ../../../include/uapi/linux/bpf.h:11:
In file included from ./include/uapi/linux/types.h:5:
/usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
file not found
#include 


-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

On Mon, Mar 26, 2018 at 08:02:30PM -0700, Alexei Starovoitov wrote:
> On Tue, Mar 27, 2018 at 10:20:10AM +0800, Du, Changbin wrote:
> > On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> > > On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> > > > Signed-off-by: Changbin Du <changbin...@intel.com>
> > > > ---
> > > >  tools/testing/selftests/bpf/Makefile | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/bpf/Makefile 
> > > > b/tools/testing/selftests/bpf/Makefile
> > > > index 5c43c18..dc0fdc8 100644
> > > > --- a/tools/testing/selftests/bpf/Makefile
> > > > +++ b/tools/testing/selftests/bpf/Makefile
> > > > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> > > >GENFLAGS := -DHAVE_GENHDR
> > > >  endif
> > > >  
> > > > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> > > > -I../../../include
> > > > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > > > + -I../../../include -I../../../../usr/include
> > > >  LDLIBS += -lcap -lelf -lrt -lpthread
> > > >  
> > > >  # Order correspond to 'make run_tests' order
> > > > @@ -62,7 +63,7 @@ else
> > > >CPU ?= generic
> > > >  endif
> > > >  
> > > > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > > > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> > > > -I../../../../usr/include \
> > > >   -Wno-compare-distinct-pointer-types
> > > 
> > > Nack.
> > > I suspect that will break the build for everyone else who's doing it in 
> > > the directory
> > > itself instead of the outer one.
> > >
> > 
> > This one? But I didn't see any problem.
> 
> because the build was lucky and additional path ../../../../usr/include 
> didn't point
> to a valid dir?
I am sorry but I don't understand why you mean *lucky*. Of cause, the path is 
valid.

> Please test with in-source and out-of-source builds.
> 
agree.

-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

On Mon, Mar 26, 2018 at 08:02:30PM -0700, Alexei Starovoitov wrote:
> On Tue, Mar 27, 2018 at 10:20:10AM +0800, Du, Changbin wrote:
> > On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> > > On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> > > > Signed-off-by: Changbin Du 
> > > > ---
> > > >  tools/testing/selftests/bpf/Makefile | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/bpf/Makefile 
> > > > b/tools/testing/selftests/bpf/Makefile
> > > > index 5c43c18..dc0fdc8 100644
> > > > --- a/tools/testing/selftests/bpf/Makefile
> > > > +++ b/tools/testing/selftests/bpf/Makefile
> > > > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> > > >GENFLAGS := -DHAVE_GENHDR
> > > >  endif
> > > >  
> > > > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> > > > -I../../../include
> > > > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > > > + -I../../../include -I../../../../usr/include
> > > >  LDLIBS += -lcap -lelf -lrt -lpthread
> > > >  
> > > >  # Order correspond to 'make run_tests' order
> > > > @@ -62,7 +63,7 @@ else
> > > >CPU ?= generic
> > > >  endif
> > > >  
> > > > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > > > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> > > > -I../../../../usr/include \
> > > >   -Wno-compare-distinct-pointer-types
> > > 
> > > Nack.
> > > I suspect that will break the build for everyone else who's doing it in 
> > > the directory
> > > itself instead of the outer one.
> > >
> > 
> > This one? But I didn't see any problem.
> 
> because the build was lucky and additional path ../../../../usr/include 
> didn't point
> to a valid dir?
I am sorry but I don't understand why you mean *lucky*. Of cause, the path is 
valid.

> Please test with in-source and out-of-source builds.
> 
agree.

-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

Hi Starovoitov,

This one does have the issue you mentioned.
[PATCH 2/4] selftests/gpio: fix paths in Makefile

And can be fixed by:

--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0

+OUTPUT ?= $(shell pwd)
 TEST_PROGS := gpio-mockup.sh
 TEST_FILES := gpio-mockup-sysfs.sh $(BINARIES)
 BINARIES := gpio-mockup-chardev
@@ -24,7 +25,7 @@ LDLIBS += -lmount -I/usr/include/libmount
 $(BINARIES): gpio-utils.o ../../../../usr/include/linux/gpio.h

 gpio-utils.o:
-   make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
+   make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) OUTPUT=$(OUTPUT)/ -C 
../../../gpio

 ../../../../usr/include/linux/gpio.h:


I will update it later.

On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du <changbin...@intel.com>
> > 
> > This patch fixed below errors of missing head files.
> > 
> > tools/testing/selftests$ make
> > ...
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf//test_pkt_access.o
> > In file included from test_pkt_access.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> >  #include 
> >  ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_xdp.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf//test_xdp.o
> > In file included from test_xdp.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> >  #include 
> >  ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_l4lb.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf//test_l4lb.o
> > In file included from test_l4lb.c:10:
> > In file included from /usr/include/linux/pkt_cls.h:4:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> >  #include 
> >  ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_tcp_estats.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf//test_tcp_estats.o
> > In file included from test_tcp_estats.c:35:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> >  #include 
> > ...
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > ---
> >  tools/testing/selftests/bpf/Makefile | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/Makefile 
> > b/tools/testing/selftests/bpf/Makefile
> > index 5c43c18..dc0fdc8 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >GENFLAGS := -DHAVE_GENHDR
> >  endif
> >  
> > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> > -I../../../include
> > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > + -I../../../include -I../../../../usr/include
> >  LDLIBS += -lcap -lelf -lrt -lpthread
> >  
> >  # Order correspond to 'make run_tests' order
> > @@ -62,7 +63,7 @@ else
> >CPU ?= generic
> >  endif
> >  
> > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> > -I../../../../usr/include \
> >   -Wno-compare-distinct-pointer-types
> 
> Nack.
> I suspect that will break the build for everyone else who's doing it in the 
> directory
> itself instead of the outer one.
> 

-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

Hi Starovoitov,

This one does have the issue you mentioned.
[PATCH 2/4] selftests/gpio: fix paths in Makefile

And can be fixed by:

--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0

+OUTPUT ?= $(shell pwd)
 TEST_PROGS := gpio-mockup.sh
 TEST_FILES := gpio-mockup-sysfs.sh $(BINARIES)
 BINARIES := gpio-mockup-chardev
@@ -24,7 +25,7 @@ LDLIBS += -lmount -I/usr/include/libmount
 $(BINARIES): gpio-utils.o ../../../../usr/include/linux/gpio.h

 gpio-utils.o:
-   make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
+   make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) OUTPUT=$(OUTPUT)/ -C 
../../../gpio

 ../../../../usr/include/linux/gpio.h:


I will update it later.

On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > This patch fixed below errors of missing head files.
> > 
> > tools/testing/selftests$ make
> > ...
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf//test_pkt_access.o
> > In file included from test_pkt_access.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> >  #include 
> >  ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_xdp.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf//test_xdp.o
> > In file included from test_xdp.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> >  #include 
> >  ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_l4lb.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf//test_l4lb.o
> > In file included from test_l4lb.c:10:
> > In file included from /usr/include/linux/pkt_cls.h:4:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> >  #include 
> >  ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi 
> > -Wno-compare-distinct-pointer-types \
> >  -O2 -target bpf -emit-llvm -c test_tcp_estats.c -o - |  \
> > llc -march=bpf -mcpu=generic -filetype=obj -o 
> > /home/changbin/work/linux/tools/testing/selftests/bpf//test_tcp_estats.o
> > In file included from test_tcp_estats.c:35:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' 
> > file not found
> >  #include 
> > ...
> > 
> > Signed-off-by: Changbin Du 
> > ---
> >  tools/testing/selftests/bpf/Makefile | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/Makefile 
> > b/tools/testing/selftests/bpf/Makefile
> > index 5c43c18..dc0fdc8 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >GENFLAGS := -DHAVE_GENHDR
> >  endif
> >  
> > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> > -I../../../include
> > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > + -I../../../include -I../../../../usr/include
> >  LDLIBS += -lcap -lelf -lrt -lpthread
> >  
> >  # Order correspond to 'make run_tests' order
> > @@ -62,7 +63,7 @@ else
> >CPU ?= generic
> >  endif
> >  
> > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> > -I../../../../usr/include \
> >   -Wno-compare-distinct-pointer-types
> 
> Nack.
> I suspect that will break the build for everyone else who's doing it in the 
> directory
> itself instead of the outer one.
> 

-- 
Thanks,
Changbin Du

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > ---
> >  tools/testing/selftests/bpf/Makefile | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/Makefile 
> > b/tools/testing/selftests/bpf/Makefile
> > index 5c43c18..dc0fdc8 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >GENFLAGS := -DHAVE_GENHDR
> >  endif
> >  
> > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> > -I../../../include
> > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > + -I../../../include -I../../../../usr/include
> >  LDLIBS += -lcap -lelf -lrt -lpthread
> >  
> >  # Order correspond to 'make run_tests' order
> > @@ -62,7 +63,7 @@ else
> >CPU ?= generic
> >  endif
> >  
> > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> > -I../../../../usr/include \
> >   -Wno-compare-distinct-pointer-types
> 
> Nack.
> I suspect that will break the build for everyone else who's doing it in the 
> directory
> itself instead of the outer one.
>

This one? But I didn't see any problem.

changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make
make -C ../../../lib/bpf 
OUTPUT=/home/changbin/work/linux/tools/testing/selftests/bpf/
make[1]: Entering directory '/home/changbin/work/linux/tools/lib/bpf'
  HOSTCC   /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep.o
  HOSTLD   /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep-in.o
  LINK /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep
  CC   /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.o
  CC   /home/changbin/work/linux/tools/testing/selftests/bpf/bpf.o
  CC   /home/changbin/work/linux/tools/testing/selftests/bpf/nlattr.o
  LD   /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf-in.o
  LINK /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a
  LINK /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.so
make[1]: Leaving directory '/home/changbin/work/linux/tools/lib/bpf'
make -C ../../../lib/bpf 
OUTPUT=/home/changbin/work/linux/tools/testing/selftests/bpf/
make[1]: Entering directory '/home/changbin/work/linux/tools/lib/bpf'
make[1]: Leaving directory '/home/changbin/work/linux/tools/lib/bpf'
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_verifier.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_verifier
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_tag.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_tag
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_maps.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_maps
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_lru_map.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_lru_map
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_lpm_map.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_lpm_map
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_progs.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_progs
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../..

Re: [PATCH 4/4] selftests/bpf: fix compiling errors

[Du, Changbin]

On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin...@intel.com wrote:
> > Signed-off-by: Changbin Du 
> > ---
> >  tools/testing/selftests/bpf/Makefile | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/Makefile 
> > b/tools/testing/selftests/bpf/Makefile
> > index 5c43c18..dc0fdc8 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> >GENFLAGS := -DHAVE_GENHDR
> >  endif
> >  
> > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) 
> > -I../../../include
> > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > + -I../../../include -I../../../../usr/include
> >  LDLIBS += -lcap -lelf -lrt -lpthread
> >  
> >  # Order correspond to 'make run_tests' order
> > @@ -62,7 +63,7 @@ else
> >CPU ?= generic
> >  endif
> >  
> > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi 
> > -I../../../../usr/include \
> >   -Wno-compare-distinct-pointer-types
> 
> Nack.
> I suspect that will break the build for everyone else who's doing it in the 
> directory
> itself instead of the outer one.
>

This one? But I didn't see any problem.

changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make
make -C ../../../lib/bpf 
OUTPUT=/home/changbin/work/linux/tools/testing/selftests/bpf/
make[1]: Entering directory '/home/changbin/work/linux/tools/lib/bpf'
  HOSTCC   /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep.o
  HOSTLD   /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep-in.o
  LINK /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep
  CC   /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.o
  CC   /home/changbin/work/linux/tools/testing/selftests/bpf/bpf.o
  CC   /home/changbin/work/linux/tools/testing/selftests/bpf/nlattr.o
  LD   /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf-in.o
  LINK /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a
  LINK /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.so
make[1]: Leaving directory '/home/changbin/work/linux/tools/lib/bpf'
make -C ../../../lib/bpf 
OUTPUT=/home/changbin/work/linux/tools/testing/selftests/bpf/
make[1]: Entering directory '/home/changbin/work/linux/tools/lib/bpf'
make[1]: Leaving directory '/home/changbin/work/linux/tools/lib/bpf'
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_verifier.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_verifier
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_tag.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_tag
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_maps.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_maps
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_lru_map.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_lru_map
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_lpm_map.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_lpm_map
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_progs.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_progs
gcc -Wall -O2 -I../../../include/uapi -I../../../lib 
-I../../../../include/generated -DHAVE_GENHDR -I../../../include 
-I../../../../usr/includetest_align.c 
/home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c 
-lcap -lelf -lrt -lpthread -o 
/home/changbin/work/linux/tools/testing/selftests/bpf/test_align
gcc -Wall -O2 

Re: [PATCH] perf trace: remove redundant ')'

[Du, Changbin]

Hi Arnaldo, How about this simple one? Thanks.

On Tue, Mar 13, 2018 at 06:40:01PM +0800, changbin...@intel.com wrote:
> From: Changbin Du <changbin...@intel.com>
> 
> There is a redundant ')' at the tail of each event. So remove it.
> $ sudo perf trace --no-syscalls -e 'kmem:*' -a
>899.342 kmem:kfree:(vfs_writev+0xb9) call_site=9c453979 ptr=(nil))
>899.344 kmem:kfree:(___sys_recvmsg+0x188) call_site=9c9b8b88 
> ptr=(nil))
> 
> Signed-off-by: Changbin Du <changbin...@intel.com>
> ---
>  tools/perf/builtin-trace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index e7f1b18..7273f5f 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1959,7 +1959,7 @@ static int trace__event_handler(struct trace *trace, 
> struct perf_evsel *evsel,
> trace->output);
>   }
>  
> - fprintf(trace->output, ")\n");
> + fprintf(trace->output, "\n");
>  
>   if (callchain_ret > 0)
>   trace__fprintf_callchain(trace, sample);
> -- 
> 2.7.4
> 

-- 
Thanks,
Changbin Du

Re: [PATCH] perf trace: remove redundant ')'

[Du, Changbin]

Hi Arnaldo, How about this simple one? Thanks.

On Tue, Mar 13, 2018 at 06:40:01PM +0800, changbin...@intel.com wrote:
> From: Changbin Du 
> 
> There is a redundant ')' at the tail of each event. So remove it.
> $ sudo perf trace --no-syscalls -e 'kmem:*' -a
>899.342 kmem:kfree:(vfs_writev+0xb9) call_site=9c453979 ptr=(nil))
>899.344 kmem:kfree:(___sys_recvmsg+0x188) call_site=9c9b8b88 
> ptr=(nil))
> 
> Signed-off-by: Changbin Du 
> ---
>  tools/perf/builtin-trace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index e7f1b18..7273f5f 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1959,7 +1959,7 @@ static int trace__event_handler(struct trace *trace, 
> struct perf_evsel *evsel,
> trace->output);
>   }
>  
> - fprintf(trace->output, ")\n");
> + fprintf(trace->output, "\n");
>  
>   if (callchain_ret > 0)
>   trace__fprintf_callchain(trace, sample);
> -- 
> 2.7.4
> 

-- 
Thanks,
Changbin Du

[Q] How does linux kernel lockdep record lock-class dependency?

[Du, Changbin]

Hello everyone,
I got a warning as below which is a AB-BA deadlock issue. But I don't understand
how the 'existing dependency' happened.

It looks like: kvm_read_guest() held (>mmap_sem), then reading userspace 
memory
(which is not ready yet) caused page_fault() invoked, then in i915_gem_fault()
it tries to hold (>struct_mutex).

But this sequence must haven't happened. Otherwise, double-lock already happed,
since intel_vgpu_create_workload() has held (>struct_mutex) already:

  (>struct_mutex)->(>mmap_sem)->(>struct_mutex)

So how could lockdep find such 'existing dependency'? Thanks!

[  163.179109] ==
[  163.185306] WARNING: possible circular locking dependency detected
[  163.191504] 4.16.0-rc5+ #44 Tainted: G U
[  163.196655] --
[  163.202854] qemu-system-x86/4514 is trying to acquire lock:
[  163.208443]  (>mmap_sem){}, at: [] 
__might_fault+0x36/0x80
[  163.216230]
   but task is already holding lock:
[  163.222090]  (>struct_mutex){+.+.}, at: [] 
copy_gma_to_hva+0xe5/0x140 [i915]
[  163.231205]
   which lock already depends on the new lock.

[  163.239421]
   the existing dependency chain (in reverse order) is:
[  163.246925]
   -> #1 (>struct_mutex){+.+.}:
[  163.252792]i915_mutex_lock_interruptible+0x66/0x170 [i915]
[  163.259005]i915_gem_fault+0x1e0/0x630 [i915]
[  163.263985]__do_fault+0x19/0xed
[  163.267830]__handle_mm_fault+0x9fa/0x1140
[  163.272550]handle_mm_fault+0x1a7/0x390
[  163.277006]__do_page_fault+0x286/0x530
[  163.281462]page_fault+0x45/0x50
[  163.285307]
   -> #0 (>mmap_sem){}:
[  163.290722]__might_fault+0x60/0x80
[  163.294839]__kvm_read_guest_page+0x3d/0x80 [kvm]
[  163.300173]kvm_read_guest+0x47/0x80 [kvm]
[  163.304891]kvmgt_rw_gpa+0x9d/0x110 [kvmgt]
[  163.309714]intel_gvt_scan_and_shadow_workload+0x1be/0x480 [i915]
[  163.316448]intel_vgpu_create_workload+0x3d9/0x550 [i915]
[  163.322488]intel_vgpu_submit_execlist+0xc0/0x2a0 [i915]
[  163.328440]elsp_mmio_write+0xcb/0x140 [i915]
[  163.333448]intel_vgpu_mmio_reg_rw+0x250/0x4f0 [i915]
[  163.339138]intel_vgpu_emulate_mmio_write+0xaa/0x240 [i915]
[  163.345337]intel_vgpu_rw+0x200/0x250 [kvmgt]
[  163.350319]intel_vgpu_write+0x164/0x1f0 [kvmgt]
[  163.38]__vfs_write+0x33/0x170
[  163.359580]vfs_write+0xc5/0x1c0
[  163.363427]SyS_pwrite64+0x90/0xb0
[  163.367447]do_syscall_64+0x70/0x1c0
[  163.371642]entry_SYSCALL_64_after_hwframe+0x42/0xb7
[  163.377230]
   other info that might help us debug this:

[  163.385258]  Possible unsafe locking scenario:

[  163.391196]CPU0CPU1
[  163.395737]
[  163.400280]   lock(>struct_mutex);
[  163.404125]lock(>mmap_sem);
[  163.410062]lock(>struct_mutex);
[  163.416436]   lock(>mmap_sem);
[  163.419846]
*** DEADLOCK ***

[  163.425780] 3 locks held by qemu-system-x86/4514:
[  163.430496]  #0:  (>lock){+.+.}, at: [] 
intel_vgpu_emulate_mmio_write+0x64/0x240 [i915]
[  163.440544]  #1:  (>struct_mutex){+.+.}, at: [] 
copy_gma_to_hva+0xe5/0x140 [i915]
[  163.450068]  #2:  (>srcu){}, at: [] 
kvmgt_rw_gpa+0x4c/0x110 [kvmgt]
[  163.458721]
   stack backtrace:
[  163.463097] CPU: 0 PID: 4514 Comm: qemu-system-x86 Tainted: G U  
 4.16.0-rc5+ #44
[  163.471663] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.2.8 
01/26/2016
[  163.479093] Call Trace:
[  163.481547]  dump_stack+0x7c/0xbe
[  163.484872]  print_circular_bug.isra.33+0x21b/0x228
[  163.489765]  __lock_acquire+0xf7d/0x1470
[  163.493700]  ? lock_acquire+0xec/0x1e0
[  163.497459]  lock_acquire+0xec/0x1e0
[  163.501046]  ? __might_fault+0x36/0x80
[  163.504805]  __might_fault+0x60/0x80
[  163.508389]  ? __might_fault+0x36/0x80
[  163.512155]  __kvm_read_guest_page+0x3d/0x80 [kvm]
[  163.516966]  kvm_read_guest+0x47/0x80 [kvm]
[  163.521161]  kvmgt_rw_gpa+0x9d/0x110 [kvmgt]
[  163.525459]  intel_gvt_scan_and_shadow_workload+0x1be/0x480 [i915]
[  163.531675]  intel_vgpu_create_workload+0x3d9/0x550 [i915]
[  163.537192]  intel_vgpu_submit_execlist+0xc0/0x2a0 [i915]
[  163.542621]  elsp_mmio_write+0xcb/0x140 [i915]
[  163.547093]  intel_vgpu_mmio_reg_rw+0x250/0x4f0 [i915]
[  163.552261]  intel_vgpu_emulate_mmio_write+0xaa/0x240 [i915]
[  163.557938]  intel_vgpu_rw+0x200/0x250 [kvmgt]
[  163.562396]  intel_vgpu_write+0x164/0x1f0 [kvmgt]
[  163.567114]  __vfs_write+0x33/0x170
[  163.570614]  ? common_file_perm+0x68/0x250
[  163.574723]  ? security_file_permission+0x36/0xb0
[  163.579440]  

[Q] How does linux kernel lockdep record lock-class dependency?

[Du, Changbin]

Hello everyone,
I got a warning as below which is a AB-BA deadlock issue. But I don't understand
how the 'existing dependency' happened.

It looks like: kvm_read_guest() held (>mmap_sem), then reading userspace 
memory
(which is not ready yet) caused page_fault() invoked, then in i915_gem_fault()
it tries to hold (>struct_mutex).

But this sequence must haven't happened. Otherwise, double-lock already happed,
since intel_vgpu_create_workload() has held (>struct_mutex) already:

  (>struct_mutex)->(>mmap_sem)->(>struct_mutex)

So how could lockdep find such 'existing dependency'? Thanks!

[  163.179109] ==
[  163.185306] WARNING: possible circular locking dependency detected
[  163.191504] 4.16.0-rc5+ #44 Tainted: G U
[  163.196655] --
[  163.202854] qemu-system-x86/4514 is trying to acquire lock:
[  163.208443]  (>mmap_sem){}, at: [] 
__might_fault+0x36/0x80
[  163.216230]
   but task is already holding lock:
[  163.222090]  (>struct_mutex){+.+.}, at: [] 
copy_gma_to_hva+0xe5/0x140 [i915]
[  163.231205]
   which lock already depends on the new lock.

[  163.239421]
   the existing dependency chain (in reverse order) is:
[  163.246925]
   -> #1 (>struct_mutex){+.+.}:
[  163.252792]i915_mutex_lock_interruptible+0x66/0x170 [i915]
[  163.259005]i915_gem_fault+0x1e0/0x630 [i915]
[  163.263985]__do_fault+0x19/0xed
[  163.267830]__handle_mm_fault+0x9fa/0x1140
[  163.272550]handle_mm_fault+0x1a7/0x390
[  163.277006]__do_page_fault+0x286/0x530
[  163.281462]page_fault+0x45/0x50
[  163.285307]
   -> #0 (>mmap_sem){}:
[  163.290722]__might_fault+0x60/0x80
[  163.294839]__kvm_read_guest_page+0x3d/0x80 [kvm]
[  163.300173]kvm_read_guest+0x47/0x80 [kvm]
[  163.304891]kvmgt_rw_gpa+0x9d/0x110 [kvmgt]
[  163.309714]intel_gvt_scan_and_shadow_workload+0x1be/0x480 [i915]
[  163.316448]intel_vgpu_create_workload+0x3d9/0x550 [i915]
[  163.322488]intel_vgpu_submit_execlist+0xc0/0x2a0 [i915]
[  163.328440]elsp_mmio_write+0xcb/0x140 [i915]
[  163.333448]intel_vgpu_mmio_reg_rw+0x250/0x4f0 [i915]
[  163.339138]intel_vgpu_emulate_mmio_write+0xaa/0x240 [i915]
[  163.345337]intel_vgpu_rw+0x200/0x250 [kvmgt]
[  163.350319]intel_vgpu_write+0x164/0x1f0 [kvmgt]
[  163.38]__vfs_write+0x33/0x170
[  163.359580]vfs_write+0xc5/0x1c0
[  163.363427]SyS_pwrite64+0x90/0xb0
[  163.367447]do_syscall_64+0x70/0x1c0
[  163.371642]entry_SYSCALL_64_after_hwframe+0x42/0xb7
[  163.377230]
   other info that might help us debug this:

[  163.385258]  Possible unsafe locking scenario:

[  163.391196]CPU0CPU1
[  163.395737]
[  163.400280]   lock(>struct_mutex);
[  163.404125]lock(>mmap_sem);
[  163.410062]lock(>struct_mutex);
[  163.416436]   lock(>mmap_sem);
[  163.419846]
*** DEADLOCK ***

[  163.425780] 3 locks held by qemu-system-x86/4514:
[  163.430496]  #0:  (>lock){+.+.}, at: [] 
intel_vgpu_emulate_mmio_write+0x64/0x240 [i915]
[  163.440544]  #1:  (>struct_mutex){+.+.}, at: [] 
copy_gma_to_hva+0xe5/0x140 [i915]
[  163.450068]  #2:  (>srcu){}, at: [] 
kvmgt_rw_gpa+0x4c/0x110 [kvmgt]
[  163.458721]
   stack backtrace:
[  163.463097] CPU: 0 PID: 4514 Comm: qemu-system-x86 Tainted: G U  
 4.16.0-rc5+ #44
[  163.471663] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.2.8 
01/26/2016
[  163.479093] Call Trace:
[  163.481547]  dump_stack+0x7c/0xbe
[  163.484872]  print_circular_bug.isra.33+0x21b/0x228
[  163.489765]  __lock_acquire+0xf7d/0x1470
[  163.493700]  ? lock_acquire+0xec/0x1e0
[  163.497459]  lock_acquire+0xec/0x1e0
[  163.501046]  ? __might_fault+0x36/0x80
[  163.504805]  __might_fault+0x60/0x80
[  163.508389]  ? __might_fault+0x36/0x80
[  163.512155]  __kvm_read_guest_page+0x3d/0x80 [kvm]
[  163.516966]  kvm_read_guest+0x47/0x80 [kvm]
[  163.521161]  kvmgt_rw_gpa+0x9d/0x110 [kvmgt]
[  163.525459]  intel_gvt_scan_and_shadow_workload+0x1be/0x480 [i915]
[  163.531675]  intel_vgpu_create_workload+0x3d9/0x550 [i915]
[  163.537192]  intel_vgpu_submit_execlist+0xc0/0x2a0 [i915]
[  163.542621]  elsp_mmio_write+0xcb/0x140 [i915]
[  163.547093]  intel_vgpu_mmio_reg_rw+0x250/0x4f0 [i915]
[  163.552261]  intel_vgpu_emulate_mmio_write+0xaa/0x240 [i915]
[  163.557938]  intel_vgpu_rw+0x200/0x250 [kvmgt]
[  163.562396]  intel_vgpu_write+0x164/0x1f0 [kvmgt]
[  163.567114]  __vfs_write+0x33/0x170
[  163.570614]  ? common_file_perm+0x68/0x250
[  163.574723]  ? security_file_permission+0x36/0xb0
[  163.579440]  

Re: [PATCH 00/17] Include linux trace docs to Sphinx TOC tree

[Du, Changbin]

On Wed, Mar 07, 2018 at 10:46:49AM -0700, Jonathan Corbet wrote:
> On Tue, 27 Feb 2018 17:43:37 -0500
> Steven Rostedt <rost...@goodmis.org> wrote:
> 
> > On Tue, 27 Feb 2018 17:34:22 +0800
> > "Du, Changbin" <changbin...@intel.com> wrote:
> >
> > > Ten days past, will you accept this serias? Thank you!
> > 
> > Currently I'm very overloaded with other code that needs to get done
> > ASAP, and I need to balance what is critical and what is not. I don't
> > have time to review this, as this isn't critical, and can wait.
> > 
> > If Jon can review it to make sure that it doesn't change the
> > readability of the text, then I'll trust his judgment. 
> 
> So I've spent a while working through the patches.  I think it's a
> well-done RST conversion carried out with a light hand; I do not believe
> there are any readability issues with the resulting text files.
> 
> I will note that the series adds some new build warnings:
> 
> > Documentation/trace/events.rst:45: WARNING: Inline emphasis start-string 
> > without end-string.
> > Documentation/trace/events.rst:49: WARNING: Inline emphasis start-string 
> > without end-string.
> > Documentation/trace/events.rst:193: WARNING: Inline emphasis start-string 
> > without end-string.
> > Documentation/trace/events.rst:114: WARNING: Unknown target name: "common".
> > Documentation/trace/ftrace.rst:2620: WARNING: Inline emphasis start-string 
> > without end-string.
> 
> These point to places where the resulting formatted docs are, in fact,
> incorrect.  I had to append the attached patch to the series to make those
> problems go away.  The warnings are there for a purpose!
> 
> Anyway, with that, the patch series is applied.  Thanks for helping to
> improve the docs, and my apologies for taking so long to get to this.
> 
> jon
> 
I am also appriciated for your review. And I am glade to see these docs can 
appear
in the new beautiful html documentation! Thnak you.

- changbin

Re: [PATCH 00/17] Include linux trace docs to Sphinx TOC tree

[Du, Changbin]

On Wed, Mar 07, 2018 at 10:46:49AM -0700, Jonathan Corbet wrote:
> On Tue, 27 Feb 2018 17:43:37 -0500
> Steven Rostedt  wrote:
> 
> > On Tue, 27 Feb 2018 17:34:22 +0800
> > "Du, Changbin"  wrote:
> >
> > > Ten days past, will you accept this serias? Thank you!
> > 
> > Currently I'm very overloaded with other code that needs to get done
> > ASAP, and I need to balance what is critical and what is not. I don't
> > have time to review this, as this isn't critical, and can wait.
> > 
> > If Jon can review it to make sure that it doesn't change the
> > readability of the text, then I'll trust his judgment. 
> 
> So I've spent a while working through the patches.  I think it's a
> well-done RST conversion carried out with a light hand; I do not believe
> there are any readability issues with the resulting text files.
> 
> I will note that the series adds some new build warnings:
> 
> > Documentation/trace/events.rst:45: WARNING: Inline emphasis start-string 
> > without end-string.
> > Documentation/trace/events.rst:49: WARNING: Inline emphasis start-string 
> > without end-string.
> > Documentation/trace/events.rst:193: WARNING: Inline emphasis start-string 
> > without end-string.
> > Documentation/trace/events.rst:114: WARNING: Unknown target name: "common".
> > Documentation/trace/ftrace.rst:2620: WARNING: Inline emphasis start-string 
> > without end-string.
> 
> These point to places where the resulting formatted docs are, in fact,
> incorrect.  I had to append the attached patch to the series to make those
> problems go away.  The warnings are there for a purpose!
> 
> Anyway, with that, the patch series is applied.  Thanks for helping to
> improve the docs, and my apologies for taking so long to get to this.
> 
> jon
> 
I am also appriciated for your review. And I am glade to see these docs can 
appear
in the new beautiful html documentation! Thnak you.

- changbin

Re: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

On Tue, Mar 06, 2018 at 11:17:07AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 06, 2018 at 08:53:02AM +0100, Jiri Olsa escreveu:
> > On Tue, Mar 06, 2018 at 11:37:35AM +0800, changbin...@intel.com wrote:
> > > From: Changbin Du <changbin...@intel.com>
> > > 
> > > v2:
> > >   o add a patch to move thread::shortname to thread_runtime
> > >   o add function perf_sched__process_comm() to process PERF_RECORD_COMM 
> > > event.
> > > 
> > > Changbin Du (2):
> > >   perf sched: move thread::shortname to thread_runtime
> > >   perf sched map: re-annotate shortname if thread comm changed
> > 
> > Acked-by: Jiri Olsa <jo...@kernel.org>
> 
> Thanks, applied both, the final layout for 'struct thread_runtime':
> 
> [root@jouet perf]# pahole -C thread_runtime ~/bin/perf
> struct thread_runtime {
>   u64last_time;/* 0 8 */
>   u64dt_run;   /* 8 8 */
>   u64dt_sleep; /*16 8 */
>   u64dt_iowait;/*24 8 */
>   u64dt_preempt;   /*32 8 */
>   u64dt_delay; /*40 8 */
>   u64ready_to_run; /*48 8 */
>   struct stats   run_stats;/*5640 */
>   /* --- cacheline 1 boundary (64 bytes) was 32 bytes ago --- */
>   u64total_run_time;   /*96 8 */
>   u64total_sleep_time; /*   104 8 */
>   u64total_iowait_time;/*   112 8 */
>   u64total_preempt_time;   /*   120 8 */
>   /* --- cacheline 2 boundary (128 bytes) --- */
>   u64total_delay_time; /*   128 8 */
>   intlast_state;   /*   136 4 */
>   char   shortname[3]; /*   140 3 */
>   _Bool  comm_changed; /*   143 1 */
>   u64migrations;   /*   144 8 */
> 
>   /* size: 152, cachelines: 3, members: 17 */
>   /* last cacheline: 24 bytes */
> };
> [root@jouet perf]#

Hi Arnaldo, thanks for your patient optimization for this!

-- 
Thanks,
Changbin Du

Re: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

On Tue, Mar 06, 2018 at 11:17:07AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 06, 2018 at 08:53:02AM +0100, Jiri Olsa escreveu:
> > On Tue, Mar 06, 2018 at 11:37:35AM +0800, changbin...@intel.com wrote:
> > > From: Changbin Du 
> > > 
> > > v2:
> > >   o add a patch to move thread::shortname to thread_runtime
> > >   o add function perf_sched__process_comm() to process PERF_RECORD_COMM 
> > > event.
> > > 
> > > Changbin Du (2):
> > >   perf sched: move thread::shortname to thread_runtime
> > >   perf sched map: re-annotate shortname if thread comm changed
> > 
> > Acked-by: Jiri Olsa 
> 
> Thanks, applied both, the final layout for 'struct thread_runtime':
> 
> [root@jouet perf]# pahole -C thread_runtime ~/bin/perf
> struct thread_runtime {
>   u64last_time;/* 0 8 */
>   u64dt_run;   /* 8 8 */
>   u64dt_sleep; /*16 8 */
>   u64dt_iowait;/*24 8 */
>   u64dt_preempt;   /*32 8 */
>   u64dt_delay; /*40 8 */
>   u64ready_to_run; /*48 8 */
>   struct stats   run_stats;/*5640 */
>   /* --- cacheline 1 boundary (64 bytes) was 32 bytes ago --- */
>   u64total_run_time;   /*96 8 */
>   u64total_sleep_time; /*   104 8 */
>   u64total_iowait_time;/*   112 8 */
>   u64total_preempt_time;   /*   120 8 */
>   /* --- cacheline 2 boundary (128 bytes) --- */
>   u64total_delay_time; /*   128 8 */
>   intlast_state;   /*   136 4 */
>   char   shortname[3]; /*   140 3 */
>   _Bool  comm_changed; /*   143 1 */
>   u64migrations;   /*   144 8 */
> 
>   /* size: 152, cachelines: 3, members: 17 */
>   /* last cacheline: 24 bytes */
> };
> [root@jouet perf]#

Hi Arnaldo, thanks for your patient optimization for this!

-- 
Thanks,
Changbin Du

Re: [RESEND PATCH] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

I just done final version, please check v2. Thanks for your comments!

On Mon, Mar 05, 2018 at 11:37:54PM +0100, Jiri Olsa wrote:
> On Mon, Mar 05, 2018 at 03:11:36PM +0800, Du, Changbin wrote:
> 
> SNIP
> 
> > > > on the other hand it's simple enough and looks
> > > > like generic solution would be more tricky
> > > 
> > > What about adding perf_sched__process_comm() to set it in the
> > > thread::priv?
> > >
> > I can be done, then thread->comm_changed moves to 
> > thread_runtime->comm_changed.
> > Draft code as below. It is also a little tricky.
> > 
> > +int perf_sched__process_comm(struct perf_tool *tool __maybe_unused,
> > +union perf_event *event,
> > +struct perf_sample *sample,
> > +struct machine *machine)
> > +{
> > +   struct thread *thread;
> > +   struct thread_runtime *r;
> > +
> > +   perf_event__process_comm(tool, event, sample, machine);
> > +
> > +   thread = machine__findnew_thread(machine, pid, tid);
> 
> should you use machine__find_thread in here?
> 
> > +   if (thread) {
> > +   r = thread__priv(thread);
> > +   if (r)
> > +   r->comm_changed = true;
> > +   thread__put(thread);
> > +   }
> > +}
> > +
> >  static int perf_sched__read_events(struct perf_sched *sched)
> >  {
> > const struct perf_evsel_str_handler handlers[] = {
> > @@ -3291,7 +3311,7 @@ int cmd_sched(int argc, const char **argv)
> > struct perf_sched sched = {
> > .tool = {
> > .sample  = 
> > perf_sched__process_tracepoint_sample,
> > -   .comm= perf_event__process_comm,
> > +   .comm= perf_sched__process_comm,
> > 
> > 
> > But I'd keep 'comm_changed' where 'shortname' is defined. I think they 
> > should appears
> > togother. And 'shortname' is only used by sched command, too.
> 
> they can both go to struct thread_runtime then
> 
> > 
> > So I still prefer my privous simpler change. Thanks!
> 
> I was wrong thinking that the amount of code
> making it sched specific would be biger
> 
> we're trying to keep the core structs generic,
> so this one fits better 
> 
> thanks,
> jirka

-- 
Thanks,
Changbin Du

Re: [RESEND PATCH] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

I just done final version, please check v2. Thanks for your comments!

On Mon, Mar 05, 2018 at 11:37:54PM +0100, Jiri Olsa wrote:
> On Mon, Mar 05, 2018 at 03:11:36PM +0800, Du, Changbin wrote:
> 
> SNIP
> 
> > > > on the other hand it's simple enough and looks
> > > > like generic solution would be more tricky
> > > 
> > > What about adding perf_sched__process_comm() to set it in the
> > > thread::priv?
> > >
> > I can be done, then thread->comm_changed moves to 
> > thread_runtime->comm_changed.
> > Draft code as below. It is also a little tricky.
> > 
> > +int perf_sched__process_comm(struct perf_tool *tool __maybe_unused,
> > +union perf_event *event,
> > +struct perf_sample *sample,
> > +struct machine *machine)
> > +{
> > +   struct thread *thread;
> > +   struct thread_runtime *r;
> > +
> > +   perf_event__process_comm(tool, event, sample, machine);
> > +
> > +   thread = machine__findnew_thread(machine, pid, tid);
> 
> should you use machine__find_thread in here?
> 
> > +   if (thread) {
> > +   r = thread__priv(thread);
> > +   if (r)
> > +   r->comm_changed = true;
> > +   thread__put(thread);
> > +   }
> > +}
> > +
> >  static int perf_sched__read_events(struct perf_sched *sched)
> >  {
> > const struct perf_evsel_str_handler handlers[] = {
> > @@ -3291,7 +3311,7 @@ int cmd_sched(int argc, const char **argv)
> > struct perf_sched sched = {
> > .tool = {
> > .sample  = 
> > perf_sched__process_tracepoint_sample,
> > -   .comm= perf_event__process_comm,
> > +   .comm= perf_sched__process_comm,
> > 
> > 
> > But I'd keep 'comm_changed' where 'shortname' is defined. I think they 
> > should appears
> > togother. And 'shortname' is only used by sched command, too.
> 
> they can both go to struct thread_runtime then
> 
> > 
> > So I still prefer my privous simpler change. Thanks!
> 
> I was wrong thinking that the amount of code
> making it sched specific would be biger
> 
> we're trying to keep the core structs generic,
> so this one fits better 
> 
> thanks,
> jirka

-- 
Thanks,
Changbin Du

Re: [RESEND PATCH] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

On Fri, Mar 02, 2018 at 11:43:12AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 23, 2018 at 07:40:40PM +0800, changbin...@intel.com escreveu:
> > From: Changbin Du <changbin...@intel.com>
> > 
> > This is to show the real name of thread that created via fork-exec.
> > See below example for shortname *A0*.
> 
> Can you ellaborate a bit more and perhaps provide before and after
> outputs?
> 
> - Arnaldo
>
Arnaldo, please see below diff stat.
  *A0   80393.050639 secs A0 => perf:22368
  *.   A0   80393.050748 secs .  => swapper:0
   .  *.80393.050887 secs
  *B0  .   .80393.052735 secs B0 => rcu_sched:8
  *.   .   .80393.052743 secs
   .  *C0  .80393.056264 secs C0 => kworker/2:1H:287
   .  *A0  .80393.056270 secs
   .  *D0  .80393.056769 secs D0 => ksoftirqd/2:22
-  .  *A0  .80393.056804 secs
+  .  *A0  .80393.056804 secs A0 => pi:22368
   .  *.   .80393.056854 secs


> > $ sudo ./perf sched map
> >   *A0   80393.050639 secs A0 => perf:22368
> >   *.   A0   80393.050748 secs .  => swapper:0
> >.  *.80393.050887 secs
> >   *B0  .   .80393.052735 secs B0 => rcu_sched:8
> >   *.   .   .80393.052743 secs
> >.  *C0  .80393.056264 secs C0 => kworker/2:1H:287
> >.  *A0  .80393.056270 secs
> >.  *D0  .80393.056769 secs D0 => ksoftirqd/2:22
> >.  *A0  .80393.056804 secs A0 => pi:22368
> >.  *.   .80393.056854 secs
> >   *B0  .   .80393.060727 secs
> >   ...
> > 
> > Signed-off-by: Changbin Du <changbin...@intel.com>
> > ---
> >  tools/perf/builtin-sched.c | 4 +++-
> >  tools/perf/util/thread.c   | 1 +
> >  tools/perf/util/thread.h   | 1 +
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 83283fe..53bb8df 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -1580,7 +1580,7 @@ static int map_switch_event(struct perf_sched *sched, 
> > struct perf_evsel *evsel,
> >  
> > timestamp__scnprintf_usec(timestamp, stimestamp, sizeof(stimestamp));
> > color_fprintf(stdout, color, "  %12s secs ", stimestamp);
> > -   if (new_shortname || (verbose > 0 && sched_in->tid)) {
> > +   if (new_shortname || sched_in->comm_changed || (verbose > 0 && 
> > sched_in->tid)) {
> > const char *pid_color = color;
> >  
> > if (thread__has_color(sched_in))
> > @@ -1588,6 +1588,8 @@ static int map_switch_event(struct perf_sched *sched, 
> > struct perf_evsel *evsel,
> >  
> > color_fprintf(stdout, pid_color, "%s => %s:%d",
> >sched_in->shortname, thread__comm_str(sched_in), 
> > sched_in->tid);
> > +
> > +   sched_in->comm_changed = false;
> > }
> >  
> > if (sched->map.comp && new_cpu)
> > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> > index 68b65b1..c660fe6 100644
> > --- a/tools/perf/util/thread.c
> > +++ b/tools/perf/util/thread.c
> > @@ -212,6 +212,7 @@ static int thread__set_comm(struct thread *thread, 
> > const char *str,
> > unwind__flush_access(thread);
> > }
> >  
> > +   thread->comm_changed = true;
> > thread->comm_set = true;
> >  
> > return 0;
> > diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> > index 40cfa36..b9a328b 100644
> > --- a/tools/perf/util/thread.h
> > +++ b/tools/perf/util/thread.h
> > @@ -27,6 +27,7 @@ struct thread {
> > int cpu;
> > refcount_t  refcnt;
> > charshortname[3];
> > +   boolcomm_changed;
> > boolcomm_set;
> > int comm_len;
> > booldead; /* if set thread has exited */
> > -- 
> > 2.7.4

-- 
Thanks,
Changbin Du

Re: [RESEND PATCH] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

On Fri, Mar 02, 2018 at 11:43:12AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 23, 2018 at 07:40:40PM +0800, changbin...@intel.com escreveu:
> > From: Changbin Du 
> > 
> > This is to show the real name of thread that created via fork-exec.
> > See below example for shortname *A0*.
> 
> Can you ellaborate a bit more and perhaps provide before and after
> outputs?
> 
> - Arnaldo
>
Arnaldo, please see below diff stat.
  *A0   80393.050639 secs A0 => perf:22368
  *.   A0   80393.050748 secs .  => swapper:0
   .  *.80393.050887 secs
  *B0  .   .80393.052735 secs B0 => rcu_sched:8
  *.   .   .80393.052743 secs
   .  *C0  .80393.056264 secs C0 => kworker/2:1H:287
   .  *A0  .80393.056270 secs
   .  *D0  .80393.056769 secs D0 => ksoftirqd/2:22
-  .  *A0  .80393.056804 secs
+  .  *A0  .80393.056804 secs A0 => pi:22368
   .  *.   .80393.056854 secs


> > $ sudo ./perf sched map
> >   *A0   80393.050639 secs A0 => perf:22368
> >   *.   A0   80393.050748 secs .  => swapper:0
> >.  *.80393.050887 secs
> >   *B0  .   .80393.052735 secs B0 => rcu_sched:8
> >   *.   .   .80393.052743 secs
> >.  *C0  .80393.056264 secs C0 => kworker/2:1H:287
> >.  *A0  .80393.056270 secs
> >.  *D0  .80393.056769 secs D0 => ksoftirqd/2:22
> >.  *A0  .80393.056804 secs A0 => pi:22368
> >.  *.   .80393.056854 secs
> >   *B0  .   .80393.060727 secs
> >   ...
> > 
> > Signed-off-by: Changbin Du 
> > ---
> >  tools/perf/builtin-sched.c | 4 +++-
> >  tools/perf/util/thread.c   | 1 +
> >  tools/perf/util/thread.h   | 1 +
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 83283fe..53bb8df 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -1580,7 +1580,7 @@ static int map_switch_event(struct perf_sched *sched, 
> > struct perf_evsel *evsel,
> >  
> > timestamp__scnprintf_usec(timestamp, stimestamp, sizeof(stimestamp));
> > color_fprintf(stdout, color, "  %12s secs ", stimestamp);
> > -   if (new_shortname || (verbose > 0 && sched_in->tid)) {
> > +   if (new_shortname || sched_in->comm_changed || (verbose > 0 && 
> > sched_in->tid)) {
> > const char *pid_color = color;
> >  
> > if (thread__has_color(sched_in))
> > @@ -1588,6 +1588,8 @@ static int map_switch_event(struct perf_sched *sched, 
> > struct perf_evsel *evsel,
> >  
> > color_fprintf(stdout, pid_color, "%s => %s:%d",
> >sched_in->shortname, thread__comm_str(sched_in), 
> > sched_in->tid);
> > +
> > +   sched_in->comm_changed = false;
> > }
> >  
> > if (sched->map.comp && new_cpu)
> > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> > index 68b65b1..c660fe6 100644
> > --- a/tools/perf/util/thread.c
> > +++ b/tools/perf/util/thread.c
> > @@ -212,6 +212,7 @@ static int thread__set_comm(struct thread *thread, 
> > const char *str,
> > unwind__flush_access(thread);
> > }
> >  
> > +   thread->comm_changed = true;
> > thread->comm_set = true;
> >  
> > return 0;
> > diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> > index 40cfa36..b9a328b 100644
> > --- a/tools/perf/util/thread.h
> > +++ b/tools/perf/util/thread.h
> > @@ -27,6 +27,7 @@ struct thread {
> > int cpu;
> > refcount_t  refcnt;
> > charshortname[3];
> > +   boolcomm_changed;
> > boolcomm_set;
> > int comm_len;
> > booldead; /* if set thread has exited */
> > -- 
> > 2.7.4

-- 
Thanks,
Changbin Du

Re: [RESEND PATCH] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

Hi,
On Fri, Mar 02, 2018 at 11:47:32PM +0900, Namhyung Kim wrote:
> Hi,
> 
> On Fri, Mar 02, 2018 at 12:38:45PM +0100, Jiri Olsa wrote:
> > On Fri, Mar 02, 2018 at 06:52:54PM +0800, Du, Changbin wrote:
> > > Hello, any comment?
> > 
> > sry, overlooked this one
> > 
> > SNIP
> > 
> > > > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> > > > index 68b65b1..c660fe6 100644
> > > > --- a/tools/perf/util/thread.c
> > > > +++ b/tools/perf/util/thread.c
> > > > @@ -212,6 +212,7 @@ static int thread__set_comm(struct thread 
> > > > *thread, const char *str,
> > > > unwind__flush_access(thread);
> > > > }
> > > >  
> > > > +   thread->comm_changed = true;
> > > > thread->comm_set = true;
> > > >  
> > > > return 0;
> > > > diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> > > > index 40cfa36..b9a328b 100644
> > > > --- a/tools/perf/util/thread.h
> > > > +++ b/tools/perf/util/thread.h
> > > > @@ -27,6 +27,7 @@ struct thread {
> > > > int cpu;
> > > > refcount_t  refcnt;
> > > > charshortname[3];
> > > > +   boolcomm_changed;
> > 
> > I don't like that it's in struct thread and set by generic function,
> > and just one command (sched) checks/sets it back.. I'd rather see it
> > in thread::priv area..
> 
> 100% agreed.
> 
> 
> > on the other hand it's simple enough and looks
> > like generic solution would be more tricky
> 
> What about adding perf_sched__process_comm() to set it in the
> thread::priv?
>
I can be done, then thread->comm_changed moves to thread_runtime->comm_changed.
Draft code as below. It is also a little tricky.

+int perf_sched__process_comm(struct perf_tool *tool __maybe_unused,
+union perf_event *event,
+struct perf_sample *sample,
+struct machine *machine)
+{
+   struct thread *thread;
+   struct thread_runtime *r;
+
+   perf_event__process_comm(tool, event, sample, machine);
+
+   thread = machine__findnew_thread(machine, pid, tid);
+   if (thread) {
+   r = thread__priv(thread);
+   if (r)
+   r->comm_changed = true;
+   thread__put(thread);
+   }
+}
+
 static int perf_sched__read_events(struct perf_sched *sched)
 {
const struct perf_evsel_str_handler handlers[] = {
@@ -3291,7 +3311,7 @@ int cmd_sched(int argc, const char **argv)
struct perf_sched sched = {
.tool = {
.sample  = 
perf_sched__process_tracepoint_sample,
-   .comm= perf_event__process_comm,
+   .comm= perf_sched__process_comm,


But I'd keep 'comm_changed' where 'shortname' is defined. I think they should 
appears
togother. And 'shortname' is only used by sched command, too.

So I still prefer my privous simpler change. Thanks!

> Thanks,
> Namhyung

-- 
Thanks,
Changbin Du

Re: [RESEND PATCH] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

Hi,
On Fri, Mar 02, 2018 at 11:47:32PM +0900, Namhyung Kim wrote:
> Hi,
> 
> On Fri, Mar 02, 2018 at 12:38:45PM +0100, Jiri Olsa wrote:
> > On Fri, Mar 02, 2018 at 06:52:54PM +0800, Du, Changbin wrote:
> > > Hello, any comment?
> > 
> > sry, overlooked this one
> > 
> > SNIP
> > 
> > > > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> > > > index 68b65b1..c660fe6 100644
> > > > --- a/tools/perf/util/thread.c
> > > > +++ b/tools/perf/util/thread.c
> > > > @@ -212,6 +212,7 @@ static int thread__set_comm(struct thread 
> > > > *thread, const char *str,
> > > > unwind__flush_access(thread);
> > > > }
> > > >  
> > > > +   thread->comm_changed = true;
> > > > thread->comm_set = true;
> > > >  
> > > > return 0;
> > > > diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> > > > index 40cfa36..b9a328b 100644
> > > > --- a/tools/perf/util/thread.h
> > > > +++ b/tools/perf/util/thread.h
> > > > @@ -27,6 +27,7 @@ struct thread {
> > > > int cpu;
> > > > refcount_t  refcnt;
> > > > charshortname[3];
> > > > +   boolcomm_changed;
> > 
> > I don't like that it's in struct thread and set by generic function,
> > and just one command (sched) checks/sets it back.. I'd rather see it
> > in thread::priv area..
> 
> 100% agreed.
> 
> 
> > on the other hand it's simple enough and looks
> > like generic solution would be more tricky
> 
> What about adding perf_sched__process_comm() to set it in the
> thread::priv?
>
I can be done, then thread->comm_changed moves to thread_runtime->comm_changed.
Draft code as below. It is also a little tricky.

+int perf_sched__process_comm(struct perf_tool *tool __maybe_unused,
+union perf_event *event,
+struct perf_sample *sample,
+struct machine *machine)
+{
+   struct thread *thread;
+   struct thread_runtime *r;
+
+   perf_event__process_comm(tool, event, sample, machine);
+
+   thread = machine__findnew_thread(machine, pid, tid);
+   if (thread) {
+   r = thread__priv(thread);
+   if (r)
+   r->comm_changed = true;
+   thread__put(thread);
+   }
+}
+
 static int perf_sched__read_events(struct perf_sched *sched)
 {
const struct perf_evsel_str_handler handlers[] = {
@@ -3291,7 +3311,7 @@ int cmd_sched(int argc, const char **argv)
struct perf_sched sched = {
.tool = {
.sample  = 
perf_sched__process_tracepoint_sample,
-   .comm= perf_event__process_comm,
+   .comm= perf_sched__process_comm,


But I'd keep 'comm_changed' where 'shortname' is defined. I think they should 
appears
togother. And 'shortname' is only used by sched command, too.

So I still prefer my privous simpler change. Thanks!

> Thanks,
> Namhyung

-- 
Thanks,
Changbin Du

Re: [RESEND PATCH] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

Hello, any comment?

On Fri, Feb 23, 2018 at 07:40:40PM +0800, changbin...@intel.com wrote:
> From: Changbin Du <changbin...@intel.com>
> 
> This is to show the real name of thread that created via fork-exec.
> See below example for shortname *A0*.
> 
> $ sudo ./perf sched map
>   *A0   80393.050639 secs A0 => perf:22368
>   *.   A0   80393.050748 secs .  => swapper:0
>.  *.80393.050887 secs
>   *B0  .   .80393.052735 secs B0 => rcu_sched:8
>   *.   .   .80393.052743 secs
>.  *C0  .80393.056264 secs C0 => kworker/2:1H:287
>.  *A0  .80393.056270 secs
>.  *D0  .80393.056769 secs D0 => ksoftirqd/2:22
>.  *A0  .80393.056804 secs A0 => pi:22368
>.  *.   .80393.056854 secs
>   *B0  .   .80393.060727 secs
>   ...
> 
> Signed-off-by: Changbin Du <changbin...@intel.com>
> ---
>  tools/perf/builtin-sched.c | 4 +++-
>  tools/perf/util/thread.c   | 1 +
>  tools/perf/util/thread.h   | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 83283fe..53bb8df 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1580,7 +1580,7 @@ static int map_switch_event(struct perf_sched *sched, 
> struct perf_evsel *evsel,
>  
>   timestamp__scnprintf_usec(timestamp, stimestamp, sizeof(stimestamp));
>   color_fprintf(stdout, color, "  %12s secs ", stimestamp);
> - if (new_shortname || (verbose > 0 && sched_in->tid)) {
> + if (new_shortname || sched_in->comm_changed || (verbose > 0 && 
> sched_in->tid)) {
>   const char *pid_color = color;
>  
>   if (thread__has_color(sched_in))
> @@ -1588,6 +1588,8 @@ static int map_switch_event(struct perf_sched *sched, 
> struct perf_evsel *evsel,
>  
>   color_fprintf(stdout, pid_color, "%s => %s:%d",
>  sched_in->shortname, thread__comm_str(sched_in), 
> sched_in->tid);
> +
> + sched_in->comm_changed = false;
>   }
>  
>   if (sched->map.comp && new_cpu)
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index 68b65b1..c660fe6 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -212,6 +212,7 @@ static int thread__set_comm(struct thread *thread, 
> const char *str,
>   unwind__flush_access(thread);
>   }
>  
> + thread->comm_changed = true;
>   thread->comm_set = true;
>  
>   return 0;
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 40cfa36..b9a328b 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -27,6 +27,7 @@ struct thread {
>   int cpu;
>   refcount_t  refcnt;
>   charshortname[3];
> + boolcomm_changed;
>   boolcomm_set;
>   int comm_len;
>   booldead; /* if set thread has exited */
> -- 
> 2.7.4
> 

-- 
Thanks,
Changbin Du

Re: [RESEND PATCH] perf sched map: re-annotate shortname if thread comm changed

[Du, Changbin]

Hello, any comment?

On Fri, Feb 23, 2018 at 07:40:40PM +0800, changbin...@intel.com wrote:
> From: Changbin Du 
> 
> This is to show the real name of thread that created via fork-exec.
> See below example for shortname *A0*.
> 
> $ sudo ./perf sched map
>   *A0   80393.050639 secs A0 => perf:22368
>   *.   A0   80393.050748 secs .  => swapper:0
>.  *.80393.050887 secs
>   *B0  .   .80393.052735 secs B0 => rcu_sched:8
>   *.   .   .80393.052743 secs
>.  *C0  .80393.056264 secs C0 => kworker/2:1H:287
>.  *A0  .80393.056270 secs
>.  *D0  .80393.056769 secs D0 => ksoftirqd/2:22
>.  *A0  .80393.056804 secs A0 => pi:22368
>.  *.   .80393.056854 secs
>   *B0  .   .80393.060727 secs
>   ...
> 
> Signed-off-by: Changbin Du 
> ---
>  tools/perf/builtin-sched.c | 4 +++-
>  tools/perf/util/thread.c   | 1 +
>  tools/perf/util/thread.h   | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 83283fe..53bb8df 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1580,7 +1580,7 @@ static int map_switch_event(struct perf_sched *sched, 
> struct perf_evsel *evsel,
>  
>   timestamp__scnprintf_usec(timestamp, stimestamp, sizeof(stimestamp));
>   color_fprintf(stdout, color, "  %12s secs ", stimestamp);
> - if (new_shortname || (verbose > 0 && sched_in->tid)) {
> + if (new_shortname || sched_in->comm_changed || (verbose > 0 && 
> sched_in->tid)) {
>   const char *pid_color = color;
>  
>   if (thread__has_color(sched_in))
> @@ -1588,6 +1588,8 @@ static int map_switch_event(struct perf_sched *sched, 
> struct perf_evsel *evsel,
>  
>   color_fprintf(stdout, pid_color, "%s => %s:%d",
>  sched_in->shortname, thread__comm_str(sched_in), 
> sched_in->tid);
> +
> + sched_in->comm_changed = false;
>   }
>  
>   if (sched->map.comp && new_cpu)
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index 68b65b1..c660fe6 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -212,6 +212,7 @@ static int thread__set_comm(struct thread *thread, 
> const char *str,
>   unwind__flush_access(thread);
>   }
>  
> + thread->comm_changed = true;
>   thread->comm_set = true;
>  
>   return 0;
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 40cfa36..b9a328b 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -27,6 +27,7 @@ struct thread {
>   int cpu;
>   refcount_t  refcnt;
>   charshortname[3];
> + boolcomm_changed;
>   boolcomm_set;
>   int comm_len;
>   booldead; /* if set thread has exited */
> -- 
> 2.7.4
> 

-- 
Thanks,
Changbin Du

Re: [PATCH v2] tracing/power: Polish the tracepoints cpu_idle and cpu_frequency

[Du, Changbin]

On Fri, Mar 02, 2018 at 11:39:16AM +0100, Rafael J. Wysocki wrote:
> On 3/2/2018 11:15 AM, Du, Changbin wrote:
> > On Fri, Mar 02, 2018 at 11:18:10AM +0100, Rafael J. Wysocki wrote:
> > > On Fri, Mar 2, 2018 at 10:41 AM, Du, Changbin <changbin...@intel.com> 
> > > wrote:
> > > > > > > That rather isn't the case if negative values are ever passed to 
> > > > > > > the
> > > > > > > tracepoint, right?
> > > > > > > 
> > > > > > yes.
> > > > > > > Which seems to be the reason why you want to make this change, 
> > > > > > > isn't it?
> > > > > > > 
> > > > > > yes, to improve readability.
> > > > > > 
> > > > > > > So maybe fix the code using the tracepoint(s) to avoid passing
> > > > > > > negative values to it(them)?
> > > > > > For cpu_idle event, [0, CPUIDLE_STATE_MAX) are used to index the 
> > > > > > idle state arrary,
> > > > > > so I think a appropriate value for PWR_EVENT_EXIT is -1 (defined in 
> > > > > > include/trace/events/power.h).
> > > > > > Or do you have a better idea? Thanks!
> > > > > Sorry, I'm not sure what you mean.
> > > > > 
> > > > > I'm saying that the code using the CPU PM tracepoints is not expected
> > > > > to pass -1 as the CPU number to them.  IOW, neither -1 nor its UL
> > > > > representation should ever appear in the output of these tracepoints.
> > > > > If that happens, it is a problem with the code using the tracepoints
> > > > > which needs to be fixed.  Users should not see any of these values.
> > > > This patch only changed 'state' field but cpuid. For cpu_idle event, 
> > > > 'state' is
> > > > singned value, but for cpu_frequency it is unsinged.
> > > > The cpuid is always unsinged value. So no one passes -1 as CPU number.
> > > You are right, 'state' not 'cpuid', sorry.
> > > 
> > > Negative 'state' should not be passed to these tracepoints too, though.
> > The current situtation is that 'state' can be negative for event cpu_idle 
> > :(. This
> > is why I made this change.
> > 
> And which is why I said that IMO it would be better to change the current
> situation.
> 
> Your patch makes the results of it slightly less confusing to a human reader
> of the tracepoint output, but the situation is still unchanged after it.
> 
> And what if someone has a script built around these tracepoints that knows
> how to handle the UL representation of -1, but doesn't know how to parse
> "-1"?  They would need to update the script after your change, wouldn't
> they?  And why would it be OK to inflict that work on them just to improve
> the readability of the output for humans?
>
yeah, I can guarantee all in kernel tools updated but people's private script.
For me, I just read the raw event for debug purpose. It is fair enough that 
leave
code as it was considering users' private tool based on this event.

> 
> 

-- 
Thanks,
Changbin Du

Re: [PATCH v2] tracing/power: Polish the tracepoints cpu_idle and cpu_frequency

[Du, Changbin]

On Fri, Mar 02, 2018 at 11:39:16AM +0100, Rafael J. Wysocki wrote:
> On 3/2/2018 11:15 AM, Du, Changbin wrote:
> > On Fri, Mar 02, 2018 at 11:18:10AM +0100, Rafael J. Wysocki wrote:
> > > On Fri, Mar 2, 2018 at 10:41 AM, Du, Changbin  
> > > wrote:
> > > > > > > That rather isn't the case if negative values are ever passed to 
> > > > > > > the
> > > > > > > tracepoint, right?
> > > > > > > 
> > > > > > yes.
> > > > > > > Which seems to be the reason why you want to make this change, 
> > > > > > > isn't it?
> > > > > > > 
> > > > > > yes, to improve readability.
> > > > > > 
> > > > > > > So maybe fix the code using the tracepoint(s) to avoid passing
> > > > > > > negative values to it(them)?
> > > > > > For cpu_idle event, [0, CPUIDLE_STATE_MAX) are used to index the 
> > > > > > idle state arrary,
> > > > > > so I think a appropriate value for PWR_EVENT_EXIT is -1 (defined in 
> > > > > > include/trace/events/power.h).
> > > > > > Or do you have a better idea? Thanks!
> > > > > Sorry, I'm not sure what you mean.
> > > > > 
> > > > > I'm saying that the code using the CPU PM tracepoints is not expected
> > > > > to pass -1 as the CPU number to them.  IOW, neither -1 nor its UL
> > > > > representation should ever appear in the output of these tracepoints.
> > > > > If that happens, it is a problem with the code using the tracepoints
> > > > > which needs to be fixed.  Users should not see any of these values.
> > > > This patch only changed 'state' field but cpuid. For cpu_idle event, 
> > > > 'state' is
> > > > singned value, but for cpu_frequency it is unsinged.
> > > > The cpuid is always unsinged value. So no one passes -1 as CPU number.
> > > You are right, 'state' not 'cpuid', sorry.
> > > 
> > > Negative 'state' should not be passed to these tracepoints too, though.
> > The current situtation is that 'state' can be negative for event cpu_idle 
> > :(. This
> > is why I made this change.
> > 
> And which is why I said that IMO it would be better to change the current
> situation.
> 
> Your patch makes the results of it slightly less confusing to a human reader
> of the tracepoint output, but the situation is still unchanged after it.
> 
> And what if someone has a script built around these tracepoints that knows
> how to handle the UL representation of -1, but doesn't know how to parse
> "-1"?  They would need to update the script after your change, wouldn't
> they?  And why would it be OK to inflict that work on them just to improve
> the readability of the output for humans?
>
yeah, I can guarantee all in kernel tools updated but people's private script.
For me, I just read the raw event for debug purpose. It is fair enough that 
leave
code as it was considering users' private tool based on this event.

> 
> 

-- 
Thanks,
Changbin Du