[GIT PULL] locking changes for v5.18

[GIT PULL] locking changes for v5.18

[Ingo Molnar]

Linus,

Please pull the latest locking/core git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-core-2022-03-21

   # HEAD: cd27ccfc727e99352321c0c75012ab9c5a90321e jump_label: Refactor #ifdef of struct static_key

Changes in this cycle were:

 - bitops & cpumask:
    - Always inline various generic helpers, to improve code generation,
      but also for instrumentation, found by noinstr validation.
    - Add a x86-specific cpumask_clear_cpu() helper to improve code generation

 - atomics:
    - Fix atomic64_{read_acquire,set_release} fallbacks

 - lockdep:
    - Fix /proc/lockdep output loop iteration for classes
    - Fix /proc/lockdep potential access to invalid memory
    - minor cleanups
    - Add Mark Rutland as reviewer for atomic primitives

 - jump labels:
    - Clean up the code a bit

 - misc:
    - Add __sched annotations to percpu rwsem primitives
    - Enable RT_MUTEXES on PREEMPT_RT by default
    - Stray v8086_mode() inlining fix, result of noinstr objtool validation

 Thanks,

	Ingo

------------------>
Borislav Petkov (4):
      asm-generic/bitops: Always inline all bit manipulation helpers
      cpumask: Always inline helpers which use bit manipulation functions
      cpumask: Add a x86-specific cpumask_clear_cpu() helper
      x86/ptrace: Always inline v8086_mode() for instrumentation

Mark Rutland (2):
      MAINTAINERS: add myself as reviewer for atomics
      atomics: Fix atomic64_{read_acquire,set_release} fallbacks

Masahiro Yamada (2):
      jump_label: Avoid unneeded casts in STATIC_KEY_INIT_{TRUE,FALSE}
      jump_label: Refactor #ifdef of struct static_key

Minchan Kim (1):
      locking: Add missing __sched attributes

Sebastian Andrzej Siewior (2):
      locking/local_lock: Make the empty local_lock_*() function a macro.
      locking: Enable RT_MUTEXES by default on PREEMPT_RT.

Waiman Long (2):
      locking/lockdep: Avoid potential access of invalid memory in lock_class
      locking/lockdep: Iterate lock_classes directly when reading lockdep files

Xiu Jianfeng (1):
      lockdep: Use memset_startat() helper in reinit_class()


 MAINTAINERS                                        |  1 +
 arch/x86/include/asm/cpumask.h                     | 10 +++++
 arch/x86/include/asm/ptrace.h                      |  2 +-
 include/asm-generic/bitops/instrumented-atomic.h   | 12 ++---
 .../asm-generic/bitops/instrumented-non-atomic.h   | 16 +++----
 include/linux/atomic/atomic-arch-fallback.h        | 38 +++++++++++++---
 include/linux/cpumask.h                            | 18 ++++----
 include/linux/jump_label.h                         | 13 ++----
 include/linux/local_lock_internal.h                |  6 +--
 init/Kconfig                                       |  1 +
 kernel/locking/lockdep.c                           | 43 ++++++++++--------
 kernel/locking/lockdep_internals.h                 |  6 ++-
 kernel/locking/lockdep_proc.c                      | 51 ++++++++++++++++++----
 kernel/locking/percpu-rwsem.c                      |  5 ++-
 kernel/locking/rwsem.c                             |  2 +-
 scripts/atomic/fallbacks/read_acquire              | 11 ++++-
 scripts/atomic/fallbacks/set_release               |  7 ++-
 17 files changed, 168 insertions(+), 74 deletions(-)

Re: [GIT PULL] locking changes for v5.18

[Linus Torvalds]

On Mon, Mar 21, 2022 at 4:11 AM Ingo Molnar <mingo@kernel.org> wrote:
>
> Sebastian Andrzej Siewior (2):
>       locking/local_lock: Make the empty local_lock_*() function a macro.

Grr. I noticed this too late, but this one actually breaks the build with clang.

Why?

Because it's now a macro, it doesn't use the argument at all, and you get:

    mm/page_alloc.c:131:40: error: variable 'pagesets' is not needed
and will not be emitted [-Werror,-Wunneeded-internal-declaration]
    static DEFINE_PER_CPU(struct pagesets, pagesets) = {
                                           ^

and I'm not sure why this doesn't show up with gcc, but apparently gcc
only warns about unused static functions, not unused static data.

Or maybe gcc considers it used just because somebody did a typeof on it.

I thought -tip had started checking with clang, but apparently not.

I see that the -mm tree has a fix for this, but I'm rather unhappy
that the -tip tree build checking has deteriorated so much, and clang
builds will now have a pointless build error that will cause issues
for bisect.

                   LInus

Re: [GIT PULL] locking changes for v5.18

[Borislav Petkov]

+ Sebastian.

On Tue, Mar 22, 2022 at 03:05:39PM -0700, Linus Torvalds wrote:
> On Mon, Mar 21, 2022 at 4:11 AM Ingo Molnar <mingo@kernel.org> wrote:
> >
> > Sebastian Andrzej Siewior (2):
> >       locking/local_lock: Make the empty local_lock_*() function a macro.
> 
> Grr. I noticed this too late, but this one actually breaks the build with clang.
> 
> Why?
> 
> Because it's now a macro, it doesn't use the argument at all, and you get:
> 
>     mm/page_alloc.c:131:40: error: variable 'pagesets' is not needed
> and will not be emitted [-Werror,-Wunneeded-internal-declaration]
>     static DEFINE_PER_CPU(struct pagesets, pagesets) = {
>                                            ^
> 
> and I'm not sure why this doesn't show up with gcc, but apparently gcc
> only warns about unused static functions, not unused static data.
> 
> Or maybe gcc considers it used just because somebody did a typeof on it.
> 
> I thought -tip had started checking with clang, but apparently not.

As a matter of fact, I do see this in my builds:

mm/page_alloc.c:131:40: warning: variable 'pagesets' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static DEFINE_PER_CPU(struct pagesets, pagesets) = {
                                       ^
1 warning generated.

but I dismissed it as one of those not-in-tip-area warnings. Sorry about
that, I'll try to pay more attention in the future.

> I see that the -mm tree has a fix for this, but I'm rather unhappy
> that the -tip tree build checking has deteriorated so much, and clang
> builds will now have a pointless build error that will cause issues
> for bisect.

Ah, you say build error because you have CONFIG_WERROR=y.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [GIT PULL] locking changes for v5.18

[Linus Torvalds]

On Tue, Mar 22, 2022 at 3:20 PM Borislav Petkov <bp@alien8.de> wrote:
>
> Ah, you say build error because you have CONFIG_WERROR=y.

EVERYBODY should have CONFIG_WERROR=y on at least x86-64 and other
serious architectures, unless you have some completely random
experimental (and broken) compiler.

New compiler warnings are not acceptable.

             Linus

Re: [GIT PULL] locking changes for v5.18

[Sebastian Andrzej Siewior]

On 2022-03-22 15:58:47 [-0700], Linus Torvalds wrote:
> On Tue, Mar 22, 2022 at 3:20 PM Borislav Petkov <bp@alien8.de> wrote:
> >
> > Ah, you say build error because you have CONFIG_WERROR=y.
> 
> EVERYBODY should have CONFIG_WERROR=y on at least x86-64 and other
> serious architectures, unless you have some completely random
> experimental (and broken) compiler.

I haven't seen it in my builds but it was reported earlier with a patch
   https://lkml.kernel.org/r/20220215184322.440969-1-nathan@kernel.org

which is sitting in Andrew's. I'm not completely satisfied with this
__maybe_unused, let me try to get clang and maybe I can figure something
else out.

>              Linus

Sebastian

[PATCH] locking/local_lock: Pretend to use the per-CPU variable if not needed.

[Sebastian Andrzej Siewior]

In the !RT && !LOCKDEP case the per-CPU variables aren't used. The commit
mentioned below tried to avoid the usage of this_cpu_ptr() because it
generates code, clobbers registers which is not needed.
This change generated so little reference to the variable that llvm
assumed that it is not used and created a warning.

Revert local_lock_*() to its previous static inline implementation for
type checking.
Replace this_cpu_ptr() with __ll_cpu_ptr() which points to
this_cpu_ptr() when it is used.
In the !RT && !LOCKDEP case use per_cpu_ptr(, 0) which does not leave
any code behind and llvm does not complain either. It also ensures that
it is a per-CPU pointer. The assembly output in this case is unchanged.

Fixes: 9983a9d577db4 ("locking/local_lock: Make the empty local_lock_*() function a macro.")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/local_lock_internal.h | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/linux/local_lock_internal.h b/include/linux/local_lock_internal.h
index 6d635e8306d64..e671ead5fbad5 100644
--- a/include/linux/local_lock_internal.h
+++ b/include/linux/local_lock_internal.h
@@ -24,6 +24,8 @@ typedef struct {
 	},						\
 	.owner = NULL,
 
+#define __ll_cpu_ptr(__ll_cpuptr) (this_cpu_ptr(__ll_cpuptr))
+
 static inline void local_lock_acquire(local_lock_t *l)
 {
 	lock_map_acquire(&l->dep_map);
@@ -44,9 +46,10 @@ static inline void local_lock_debug_init(local_lock_t *l)
 }
 #else /* CONFIG_DEBUG_LOCK_ALLOC */
 # define LOCAL_LOCK_DEBUG_INIT(lockname)
-# define local_lock_acquire(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
-# define local_lock_release(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
-# define local_lock_debug_init(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
+# define __ll_cpu_ptr(__ll_cpuptr) per_cpu_ptr(__ll_cpuptr, 0)
+static inline void local_lock_acquire(local_lock_t *l) { }
+static inline void local_lock_release(local_lock_t *l) { }
+static inline void local_lock_debug_init(local_lock_t *l) { }
 #endif /* !CONFIG_DEBUG_LOCK_ALLOC */
 
 #define INIT_LOCAL_LOCK(lockname)	{ LOCAL_LOCK_DEBUG_INIT(lockname) }
@@ -65,36 +68,36 @@ do {								\
 #define __local_lock(lock)					\
 	do {							\
 		preempt_disable();				\
-		local_lock_acquire(this_cpu_ptr(lock));		\
+		local_lock_acquire(__ll_cpu_ptr(lock));		\
 	} while (0)
 
 #define __local_lock_irq(lock)					\
 	do {							\
 		local_irq_disable();				\
-		local_lock_acquire(this_cpu_ptr(lock));		\
+		local_lock_acquire(__ll_cpu_ptr(lock));		\
 	} while (0)
 
 #define __local_lock_irqsave(lock, flags)			\
 	do {							\
 		local_irq_save(flags);				\
-		local_lock_acquire(this_cpu_ptr(lock));		\
+		local_lock_acquire(__ll_cpu_ptr(lock));		\
 	} while (0)
 
 #define __local_unlock(lock)					\
 	do {							\
-		local_lock_release(this_cpu_ptr(lock));		\
+		local_lock_release(__ll_cpu_ptr(lock));		\
 		preempt_enable();				\
 	} while (0)
 
 #define __local_unlock_irq(lock)				\
 	do {							\
-		local_lock_release(this_cpu_ptr(lock));		\
+		local_lock_release(__ll_cpu_ptr(lock));		\
 		local_irq_enable();				\
 	} while (0)
 
 #define __local_unlock_irqrestore(lock, flags)			\
 	do {							\
-		local_lock_release(this_cpu_ptr(lock));		\
+		local_lock_release(__ll_cpu_ptr(lock));		\
 		local_irq_restore(flags);			\
 	} while (0)
 
-- 
2.35.1

Re: [PATCH] locking/local_lock: Pretend to use the per-CPU variable if not needed.

[Linus Torvalds]

On Wed, Mar 23, 2022 at 4:09 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> Replace this_cpu_ptr() with __ll_cpu_ptr() which points to
> this_cpu_ptr() when it is used.

Ok, so that's just really ugly.

Is there really no way to just fixthis_cpu_ptr() to not generate crap
code when the result isn't used?

I get the feeling that the real problem is that on x86, we have this:

#define arch_raw_cpu_ptr(ptr)                           \
({                                                      \
        unsigned long tcp_ptr__;                        \
        asm volatile("add " __percpu_arg(1) ", %0"      \
                     : "=r" (tcp_ptr__)                 \
                     : "m" (this_cpu_off), "0" (ptr));  \
        (typeof(*(ptr)) __kernel __force *)tcp_ptr__;   \
})

and that "volatile" is just *WRONG*.

That volatile is what literally tells the compiler "you can't remove
this if it isn't used".

But there's no point to that.

So how about we

 (a) just revert commit 9983a9d577db4

 (b) remove that bogus 'volatile'

Doesn't that fix the problem?

              Linus

[PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks.

[Sebastian Andrzej Siewior]

On 2022-03-23 10:17:08 [-0700], Linus Torvalds wrote:
> I get the feeling that the real problem is that on x86, we have this:
> 
> #define arch_raw_cpu_ptr(ptr)                           \
> ({                                                      \
>         unsigned long tcp_ptr__;                        \
>         asm volatile("add " __percpu_arg(1) ", %0"      \
>                      : "=r" (tcp_ptr__)                 \
>                      : "m" (this_cpu_off), "0" (ptr));  \
>         (typeof(*(ptr)) __kernel __force *)tcp_ptr__;   \
> })
> 
> and that "volatile" is just *WRONG*.
> 
> That volatile is what literally tells the compiler "you can't remove
> this if it isn't used".

It is indeed just x86. After double checking arm/mips removes that code
properly.

> But there's no point to that.
> 
> So how about we
> 
>  (a) just revert commit 9983a9d577db4
> 
>  (b) remove that bogus 'volatile'
> 
> Doesn't that fix the problem?

The following series does that. The assembly code looks okay. In a few
simple test cases the this_cpu_ptr() usage is always created and is not
moved passed preempt_enable() statement.
The resulting vmlinux shrunk a bit. The test config lost ~2KiB:
     text      data       bss       dec      hex filename
 22533901  10722831  13963496  47220228  2d08604 vmlinux.volatile
 22531589  10722831  13971688  47226108  2d09cfc vmlinux.patched

after looking at it it was sometimes due avoiding this_cpu_ptr(),
sometimes it looked that the compiler made other decisions at the
earlier resulting to be more beneficial later on.

Sebastian

[PATCH 1/3] x86/percpu: Remove volatile from arch_raw_cpu_ptr().

[Sebastian Andrzej Siewior]

The volatile attribute in the inline assembly of arch_raw_cpu_ptr()
forces the compiler to always generate the code, even if the compiler
can decide upfront that its result is not needed.

For instance invoking __intel_pmu_disable_all(false) (like
intel_pmu_snapshot_arch_branch_stack() does) leads to loading the
address of &cpu_hw_events into the register while compiler knows that it
has no need for it. This ends up with code like:

|	movq	$cpu_hw_events, %rax			#, tcp_ptr__
|	add	%gs:this_cpu_off(%rip), %rax		# this_cpu_off, tcp_ptr__
|	xorl	%eax, %eax				# tmp93

It also creates additional code within local_lock() with !RT &&
!LOCKDEP which is not desired.

By removing the volatile attribute the compiler can place the
function freely and avoid it if it is not needed in the end.
By using the function twice the compiler properly caches only the
variable offset and always loads the CPU-offset.

Remove volatile from arch_raw_cpu_ptr().

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/include/asm/percpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index a3c33b79fb865..5d572a19a389c 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -38,7 +38,7 @@
 #define arch_raw_cpu_ptr(ptr)				\
 ({							\
 	unsigned long tcp_ptr__;			\
-	asm volatile("add " __percpu_arg(1) ", %0"	\
+	asm ("add " __percpu_arg(1) ", %0"	\
 		     : "=r" (tcp_ptr__)			\
 		     : "m" (this_cpu_off), "0" (ptr));	\
 	(typeof(*(ptr)) __kernel __force *)tcp_ptr__;	\
-- 
2.35.1

[PATCH 2/3] Revert "locking/local_lock: Make the empty local_lock_*() function a macro."

[Sebastian Andrzej Siewior]

With volatile removed from arch_raw_cpu_ptr() the compiler no longer
creates the per-CPU reference. The usage of the macro can be reverted
now.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/local_lock_internal.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/local_lock_internal.h b/include/linux/local_lock_internal.h
index 6d635e8306d64..975e33b793a77 100644
--- a/include/linux/local_lock_internal.h
+++ b/include/linux/local_lock_internal.h
@@ -44,9 +44,9 @@ static inline void local_lock_debug_init(local_lock_t *l)
 }
 #else /* CONFIG_DEBUG_LOCK_ALLOC */
 # define LOCAL_LOCK_DEBUG_INIT(lockname)
-# define local_lock_acquire(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
-# define local_lock_release(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
-# define local_lock_debug_init(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
+static inline void local_lock_acquire(local_lock_t *l) { }
+static inline void local_lock_release(local_lock_t *l) { }
+static inline void local_lock_debug_init(local_lock_t *l) { }
 #endif /* !CONFIG_DEBUG_LOCK_ALLOC */
 
 #define INIT_LOCAL_LOCK(lockname)	{ LOCAL_LOCK_DEBUG_INIT(lockname) }
-- 
2.35.1

[PATCH 3/3] Revert "mm/page_alloc: mark pagesets as __maybe_unused"

[Sebastian Andrzej Siewior]

The local_lock() is now using a proper static inline function which is
enough for llvm to accept that the variable is used.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6e0b4596cde9b..8f83e9929c7d2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -128,7 +128,7 @@ static DEFINE_MUTEX(pcp_batch_high_lock);
 struct pagesets {
 	local_lock_t lock;
 };
-static DEFINE_PER_CPU(struct pagesets, pagesets) __maybe_unused = {
+static DEFINE_PER_CPU(struct pagesets, pagesets) = {
 	.lock = INIT_LOCAL_LOCK(lock),
 };
 
-- 
2.35.1

Re: [PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks.

[Linus Torvalds]

On Thu, Mar 24, 2022 at 10:39 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> The following series does that. The assembly code looks okay. In a few
> simple test cases the this_cpu_ptr() usage is always created and is not
> moved passed preempt_enable() statement.

Hmm. I didn't think things through, and you're very correct in having
thought about preempt_enable/disable as needing barriers for that asm.

So the "volatile" does have that potential significance for that
arch_raw_cpu_ptr() in case it was what ordered it wrt preemption.

It turns out that it's all ok, because

 - even without the 'volatile', arch_raw_cpu_ptr() has an *input* that
is a memory location:

                     : "m" (this_cpu_off)

 - and prempt_{dis,en}able() fundamentally has a 'barrier()' in it

and as such it's not just random luck that the percpu thing is never
moved around preemption points. They are properly serialized even
without any 'volatile'.

But that "it's not just random luck" might be worth a comment.

So Ack on your series, but that additional comment might be worth it,
since I didn't even think of it until you mentioned it. Clearly much
too subtle.

Thanks,
              Linus

Re: [PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks.

[Peter Zijlstra]

On Thu, Mar 24, 2022 at 11:28:16AM -0700, Linus Torvalds wrote:

> So Ack on your series, but that additional comment might be worth it,
> since I didn't even think of it until you mentioned it. Clearly much
> too subtle.

Sebastian, were you going to resend these patches with that comment on,
or should I go collect them as is for post -rc1?

Re: [PATCH 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks.

[Sebastian Andrzej Siewior]

On 2022-03-28 15:55:14 [+0200], Peter Zijlstra wrote:
> On Thu, Mar 24, 2022 at 11:28:16AM -0700, Linus Torvalds wrote:
> 
> > So Ack on your series, but that additional comment might be worth it,
> > since I didn't even think of it until you mentioned it. Clearly much
> > too subtle.
> 
> Sebastian, were you going to resend these patches with that comment on,
> or should I go collect them as is for post -rc1?

Just did so. Sorry, had no time on FRI.

Sebastian

[PATCH v2 0/3] Remove volatile from arch_raw_cpu_ptr() and revert the hacks.

[Sebastian Andrzej Siewior]

Remove volatile from arch_raw_cpu_ptr() and revert the hacks.

v1…v2: Updated the comments in 1/3

Sebastian

[PATCH v2 1/3] x86/percpu: Remove volatile from arch_raw_cpu_ptr().

[Sebastian Andrzej Siewior]

The volatile attribute in the inline assembly of arch_raw_cpu_ptr()
forces the compiler to always generate the code, even if the compiler
can decide upfront that its result is not needed.

For instance invoking __intel_pmu_disable_all(false) (like
intel_pmu_snapshot_arch_branch_stack() does) leads to loading the
address of &cpu_hw_events into the register while compiler knows that it
has no need for it. This ends up with code like:

|	movq	$cpu_hw_events, %rax			#, tcp_ptr__
|	add	%gs:this_cpu_off(%rip), %rax		# this_cpu_off, tcp_ptr__
|	xorl	%eax, %eax				# tmp93

It also creates additional code within local_lock() with !RT &&
!LOCKDEP which is not desired.

By removing the volatile attribute the compiler can place the
function freely and avoid it if it is not needed in the end.
By using the function twice the compiler properly caches only the
variable offset and always loads the CPU-offset.

this_cpu_ptr() also remains properly placed within a preempt_disable()
sections because
- arch_raw_cpu_ptr() assembly has a memory input ("m" (this_cpu_off))
- prempt_{dis,en}able() fundamentally has a 'barrier()' in it

Therefore this_cpu_ptr() is already properly serialized and does not
rely on the 'volatile' attribute.

Remove volatile from arch_raw_cpu_ptr().

[ bigeasy: Added Linus' explanation why this_cpu_ptr() is not moved out
  of a preempt_disable() section without the 'volatile' attribute. ]

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/include/asm/percpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index a3c33b79fb865..5d572a19a389c 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -38,7 +38,7 @@
 #define arch_raw_cpu_ptr(ptr)				\
 ({							\
 	unsigned long tcp_ptr__;			\
-	asm volatile("add " __percpu_arg(1) ", %0"	\
+	asm ("add " __percpu_arg(1) ", %0"	\
 		     : "=r" (tcp_ptr__)			\
 		     : "m" (this_cpu_off), "0" (ptr));	\
 	(typeof(*(ptr)) __kernel __force *)tcp_ptr__;	\
-- 
2.35.1

[tip: locking/urgent] x86/percpu: Remove volatile from arch_raw_cpu_ptr().

[tip-bot2 for Sebastian Andrzej Siewior]

The following commit has been merged into the locking/urgent branch of tip:

Commit-ID:     1c1e7e3c23dd25f938302428eeb22c3dda2c3427
Gitweb:        https://git.kernel.org/tip/1c1e7e3c23dd25f938302428eeb22c3dda2c3427
Author:        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate:    Mon, 28 Mar 2022 16:58:08 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Apr 2022 09:59:38 +02:00

x86/percpu: Remove volatile from arch_raw_cpu_ptr().

The volatile attribute in the inline assembly of arch_raw_cpu_ptr()
forces the compiler to always generate the code, even if the compiler
can decide upfront that its result is not needed.

For instance invoking __intel_pmu_disable_all(false) (like
intel_pmu_snapshot_arch_branch_stack() does) leads to loading the
address of &cpu_hw_events into the register while compiler knows that it
has no need for it. This ends up with code like:

|	movq	$cpu_hw_events, %rax			#, tcp_ptr__
|	add	%gs:this_cpu_off(%rip), %rax		# this_cpu_off, tcp_ptr__
|	xorl	%eax, %eax				# tmp93

It also creates additional code within local_lock() with !RT &&
!LOCKDEP which is not desired.

By removing the volatile attribute the compiler can place the
function freely and avoid it if it is not needed in the end.
By using the function twice the compiler properly caches only the
variable offset and always loads the CPU-offset.

this_cpu_ptr() also remains properly placed within a preempt_disable()
sections because
- arch_raw_cpu_ptr() assembly has a memory input ("m" (this_cpu_off))
- prempt_{dis,en}able() fundamentally has a 'barrier()' in it

Therefore this_cpu_ptr() is already properly serialized and does not
rely on the 'volatile' attribute.

Remove volatile from arch_raw_cpu_ptr().

[ bigeasy: Added Linus' explanation why this_cpu_ptr() is not moved out
  of a preempt_disable() section without the 'volatile' attribute. ]

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20220328145810.86783-2-bigeasy@linutronix.de
---
 arch/x86/include/asm/percpu.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index a3c33b7..13c0d63 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -38,9 +38,9 @@
 #define arch_raw_cpu_ptr(ptr)				\
 ({							\
 	unsigned long tcp_ptr__;			\
-	asm volatile("add " __percpu_arg(1) ", %0"	\
-		     : "=r" (tcp_ptr__)			\
-		     : "m" (this_cpu_off), "0" (ptr));	\
+	asm ("add " __percpu_arg(1) ", %0"		\
+	     : "=r" (tcp_ptr__)				\
+	     : "m" (this_cpu_off), "0" (ptr));		\
 	(typeof(*(ptr)) __kernel __force *)tcp_ptr__;	\
 })
 #else

[PATCH v2 2/3] Revert "locking/local_lock: Make the empty local_lock_*() function a macro."

[Sebastian Andrzej Siewior]

With volatile removed from arch_raw_cpu_ptr() the compiler no longer
creates the per-CPU reference. The usage of the macro can be reverted
now.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/local_lock_internal.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/local_lock_internal.h b/include/linux/local_lock_internal.h
index 6d635e8306d64..975e33b793a77 100644
--- a/include/linux/local_lock_internal.h
+++ b/include/linux/local_lock_internal.h
@@ -44,9 +44,9 @@ static inline void local_lock_debug_init(local_lock_t *l)
 }
 #else /* CONFIG_DEBUG_LOCK_ALLOC */
 # define LOCAL_LOCK_DEBUG_INIT(lockname)
-# define local_lock_acquire(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
-# define local_lock_release(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
-# define local_lock_debug_init(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
+static inline void local_lock_acquire(local_lock_t *l) { }
+static inline void local_lock_release(local_lock_t *l) { }
+static inline void local_lock_debug_init(local_lock_t *l) { }
 #endif /* !CONFIG_DEBUG_LOCK_ALLOC */
 
 #define INIT_LOCAL_LOCK(lockname)	{ LOCAL_LOCK_DEBUG_INIT(lockname) }
-- 
2.35.1

[tip: locking/urgent] Revert "locking/local_lock: Make the empty local_lock_*() function a macro."

[tip-bot2 for Sebastian Andrzej Siewior]

The following commit has been merged into the locking/urgent branch of tip:

Commit-ID:     2d2f8f083ef29e9b7adfe5cb421368331543473f
Gitweb:        https://git.kernel.org/tip/2d2f8f083ef29e9b7adfe5cb421368331543473f
Author:        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate:    Mon, 28 Mar 2022 16:58:09 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Apr 2022 09:59:39 +02:00

Revert "locking/local_lock: Make the empty local_lock_*() function a macro."

With volatile removed from arch_raw_cpu_ptr() the compiler no longer
creates the per-CPU reference. The usage of the macro can be reverted
now.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20220328145810.86783-3-bigeasy@linutronix.de
---
 include/linux/local_lock_internal.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/local_lock_internal.h b/include/linux/local_lock_internal.h
index 6d635e8..975e33b 100644
--- a/include/linux/local_lock_internal.h
+++ b/include/linux/local_lock_internal.h
@@ -44,9 +44,9 @@ static inline void local_lock_debug_init(local_lock_t *l)
 }
 #else /* CONFIG_DEBUG_LOCK_ALLOC */
 # define LOCAL_LOCK_DEBUG_INIT(lockname)
-# define local_lock_acquire(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
-# define local_lock_release(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
-# define local_lock_debug_init(__ll)  do { typecheck(local_lock_t *, __ll); } while (0)
+static inline void local_lock_acquire(local_lock_t *l) { }
+static inline void local_lock_release(local_lock_t *l) { }
+static inline void local_lock_debug_init(local_lock_t *l) { }
 #endif /* !CONFIG_DEBUG_LOCK_ALLOC */
 
 #define INIT_LOCAL_LOCK(lockname)	{ LOCAL_LOCK_DEBUG_INIT(lockname) }

[PATCH v2 3/3] Revert "mm/page_alloc: mark pagesets as __maybe_unused"

[Sebastian Andrzej Siewior]

The local_lock() is now using a proper static inline function which is
enough for llvm to accept that the variable is used.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bdc8f60ae4622..634323b13cd2f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -128,7 +128,7 @@ static DEFINE_MUTEX(pcp_batch_high_lock);
 struct pagesets {
 	local_lock_t lock;
 };
-static DEFINE_PER_CPU(struct pagesets, pagesets) __maybe_unused = {
+static DEFINE_PER_CPU(struct pagesets, pagesets) = {
 	.lock = INIT_LOCAL_LOCK(lock),
 };
 
-- 
2.35.1

[tip: locking/urgent] Revert "mm/page_alloc: mark pagesets as __maybe_unused"

[tip-bot2 for Sebastian Andrzej Siewior]

The following commit has been merged into the locking/urgent branch of tip:

Commit-ID:     273ba85b5e8b971ed28eb5c17e1638543be9237d
Gitweb:        https://git.kernel.org/tip/273ba85b5e8b971ed28eb5c17e1638543be9237d
Author:        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate:    Mon, 28 Mar 2022 16:58:10 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Apr 2022 09:59:39 +02:00

Revert "mm/page_alloc: mark pagesets as __maybe_unused"

The local_lock() is now using a proper static inline function which is
enough for llvm to accept that the variable is used.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20220328145810.86783-4-bigeasy@linutronix.de
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2db9578..6e5b448 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -128,7 +128,7 @@ static DEFINE_MUTEX(pcp_batch_high_lock);
 struct pagesets {
 	local_lock_t lock;
 };
-static DEFINE_PER_CPU(struct pagesets, pagesets) __maybe_unused = {
+static DEFINE_PER_CPU(struct pagesets, pagesets) = {
 	.lock = INIT_LOCAL_LOCK(lock),
 };
 

[PATCH] x86/defconfig: Enable WERROR

[Borislav Petkov]

From: Borislav Petkov <bp@suse.de>

To quote Linus:

"EVERYBODY should have CONFIG_WERROR=y on at least x86-64 and other
serious architectures, unless you have some completely random
experimental (and broken) compiler.

New compiler warnings are not acceptable."

So this should make at least the most obvious and common ones not go
unnoticed.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/configs/i386_defconfig   | 1 +
 arch/x86/configs/x86_64_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
index 71124cf8630c..4b2085243aa3 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -262,3 +262,4 @@ CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
 CONFIG_EARLY_PRINTK_DBGP=y
 CONFIG_DEBUG_BOOT_PARAMS=y
 CONFIG_KALLSYMS_ALL=y
+CONFIG_WERROR=y
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index 92b1169ec90b..38c52e4eb30b 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -258,3 +258,4 @@ CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
 CONFIG_EARLY_PRINTK_DBGP=y
 CONFIG_DEBUG_BOOT_PARAMS=y
 CONFIG_KALLSYMS_ALL=y
+CONFIG_WERROR=y
-- 
2.29.2


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/defconfig: Enable WERROR

[Linus Torvalds]

On Wed, Mar 23, 2022 at 4:21 AM Borislav Petkov <bp@alien8.de> wrote:
>
> So this should make at least the most obvious and common ones not go
> unnoticed.

Ack. I'm actually surprised people use "defconfig". It's kind of
pointless and unreal on x86 where there is no realistic "default". I'm
sorry it exists at all.

But since people clearly use it - presumably for testing - I very much
think it should have WERROR on.

                Linus

Re: [PATCH] x86/defconfig: Enable WERROR

[Borislav Petkov]

On Wed, Mar 23, 2022 at 10:19:39AM -0700, Linus Torvalds wrote:
> Ack. I'm actually surprised people use "defconfig". It's kind of
> pointless and unreal on x86 where there is no realistic "default". I'm
> sorry it exists at all.

Yeah, it has been there since forever and peterz and mingo use it from
time to time, AFAIK. Also, I know people start from a defconfig and add
additional stuff they need.

Oh, and it is part of the smoke build-tests we're doing so...

> But since people clearly use it - presumably for testing - I very much
> think it should have WERROR on.

Yeah, I am aiming for a "this should be always enabled in your config
so you could use this defconfig as a start"-thing, in the hope that
WERROR=y would proliferate then.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Mar 23, 2022 at 4:21 AM Borislav Petkov <bp@alien8.de> wrote:
> >
> > So this should make at least the most obvious and common ones not go
> > unnoticed.
> 
> Ack. I'm actually surprised people use "defconfig". It's kind of
> pointless and unreal on x86 where there is no realistic "default". I'm
> sorry it exists at all.
>
> But since people clearly use it - presumably for testing - I very much 
> think it should have WERROR on.

Hm, I never saw the warning you got - I test 8 main configs on x86:

 - {x86_64|i386} X {allnoconfig,defconfig,allmodconfig,allyesconfig}

Plus an unlimited number of randconfigs. Normally no warnings are permitted 
in any of the 8 base configs - regardless of whether WERROR is enabled or 
not.

I'm using x86 defconfig as:

 - A quick to build, bootable config (on KVM) with settings that are 
   supposed to be pretty close to how the bigger x86 distros set up their 
   kernels. Defconfig builds ~5x faster than allmodconfig.

 - Reference config for kernel bloat measurements. Empirically the text 
   section of its vmlinux shrinks roughly as well as a full distro kernel 
   build - with a fraction of a build time.

 - Developer reference point & minimal test requirements. If an incoming 
   patch doesn't even build on defconfig that's a serious red flag.

 - I also use defconfig when applying individual patches or series when I 
   cannot do a full allmodconfig build for the entire series.

 - I also have kconfig scripts that enable all of the (practical...) debug 
   options in a kernel. So I often boot x86-64 defconfig, then 
   defconfig+fulldebug. Again this is much faster & practical to do than 
   allmod or allyes configs.

But beyond the WERROR oversight, bitrot has gotten to defconfig a bit - see 
attached patch. My test scripts usually enable these - but it's better to 
formalize it.

Anyway, I've put Boris's patch into x86/urgent with your Acked-by, then my 
bigger defconfig cleanup patch. We'll send a pull request for these ASAP, 
plus the local_lock cleanup once there's consensus.

Thanks,

	Ingo

================>

From: Ingo Molnar <mingo@kernel.org>
Date: Thu, 24 Mar 2022 09:18:36 +0100
Subject: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

 - Use 'make savedefconfig' to refresh & regenerate the files
 - Add in KVM boot enablers
 - Enable the cgroup features most distros rely on

Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/configs/i386_defconfig   | 50 ++++++++++++++++++++++++++-------------
 arch/x86/configs/x86_64_defconfig | 38 +++++++++++++++++++++--------
 2 files changed, 62 insertions(+), 26 deletions(-)

diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
index 4b2085243aa3..fde771df72cc 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -1,5 +1,7 @@
+CONFIG_WERROR=y
 CONFIG_SYSVIPC=y
 CONFIG_POSIX_MQUEUE=y
+CONFIG_USELIB=y
 CONFIG_AUDIT=y
 CONFIG_NO_HZ=y
 CONFIG_HIGH_RES_TIMERS=y
@@ -11,23 +13,30 @@ CONFIG_TASK_XACCT=y
 CONFIG_TASK_IO_ACCOUNTING=y
 CONFIG_LOG_BUF_SHIFT=18
 CONFIG_CGROUPS=y
+CONFIG_BLK_CGROUP=y
 CONFIG_CGROUP_SCHED=y
+CONFIG_CGROUP_PIDS=y
+CONFIG_CGROUP_RDMA=y
 CONFIG_CGROUP_FREEZER=y
+CONFIG_CGROUP_HUGETLB=y
 CONFIG_CPUSETS=y
+CONFIG_CGROUP_DEVICE=y
 CONFIG_CGROUP_CPUACCT=y
+CONFIG_CGROUP_PERF=y
+CONFIG_CGROUP_MISC=y
+CONFIG_CGROUP_DEBUG=y
 CONFIG_BLK_DEV_INITRD=y
+CONFIG_KALLSYMS_ALL=y
 # CONFIG_COMPAT_BRK is not set
 CONFIG_PROFILING=y
-# CONFIG_64BIT is not set
 CONFIG_SMP=y
-CONFIG_X86_GENERIC=y
-CONFIG_HPET_TIMER=y
+CONFIG_HYPERVISOR_GUEST=y
+CONFIG_PARAVIRT=y
+CONFIG_NR_CPUS=8
 CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
-CONFIG_X86_REBOOTFIXUPS=y
 CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
 CONFIG_X86_CPUID=y
-CONFIG_HIGHPTE=y
 CONFIG_X86_CHECK_BIOS_CORRUPTION=y
 # CONFIG_MTRR_SANITIZER is not set
 CONFIG_EFI=y
@@ -43,12 +52,15 @@ CONFIG_ACPI_BGRT=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_X86_ACPI_CPUFREQ=y
-CONFIG_EFI_VARS=y
 CONFIG_KPROBES=y
 CONFIG_JUMP_LABEL=y
+CONFIG_COMPAT_32BIT_TIME=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 CONFIG_MODULE_FORCE_UNLOAD=y
+CONFIG_BLK_CGROUP_IOLATENCY=y
+CONFIG_BLK_CGROUP_IOCOST=y
+CONFIG_BLK_CGROUP_IOPRIO=y
 CONFIG_BINFMT_MISC=y
 CONFIG_NET=y
 CONFIG_PACKET=y
@@ -103,12 +115,16 @@ CONFIG_IP6_NF_FILTER=y
 CONFIG_IP6_NF_TARGET_REJECT=y
 CONFIG_IP6_NF_MANGLE=y
 CONFIG_NET_SCHED=y
+CONFIG_NET_CLS_CGROUP=y
 CONFIG_NET_EMATCH=y
 CONFIG_NET_CLS_ACT=y
+CONFIG_CGROUP_NET_PRIO=y
 CONFIG_CFG80211=y
 CONFIG_MAC80211=y
 CONFIG_MAC80211_LEDS=y
 CONFIG_RFKILL=y
+CONFIG_NET_9P=y
+CONFIG_NET_9P_VIRTIO=y
 CONFIG_PCI=y
 CONFIG_PCIEPORTBUS=y
 CONFIG_PCI_MSI=y
@@ -119,13 +135,16 @@ CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_DEBUG_DEVRES=y
 CONFIG_CONNECTOR=y
+CONFIG_EFI_VARS=y
+CONFIG_EFI_CAPSULE_LOADER=y
 CONFIG_BLK_DEV_LOOP=y
+CONFIG_VIRTIO_BLK=y
 CONFIG_BLK_DEV_SD=y
 CONFIG_BLK_DEV_SR=y
 CONFIG_CHR_DEV_SG=y
 CONFIG_SCSI_CONSTANTS=y
 CONFIG_SCSI_SPI_ATTRS=y
-# CONFIG_SCSI_LOWLEVEL is not set
+CONFIG_SCSI_VIRTIO=y
 CONFIG_ATA=y
 CONFIG_SATA_AHCI=y
 CONFIG_ATA_PIIX=y
@@ -143,6 +162,7 @@ CONFIG_MACINTOSH_DRIVERS=y
 CONFIG_MAC_EMUMOUSEBTN=y
 CONFIG_NETDEVICES=y
 CONFIG_NETCONSOLE=y
+CONFIG_VIRTIO_NET=y
 CONFIG_BNX2=y
 CONFIG_TIGON3=y
 CONFIG_NET_TULIP=y
@@ -170,6 +190,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
 CONFIG_SERIAL_8250_DETECT_IRQ=y
 CONFIG_SERIAL_8250_RSA=y
 CONFIG_SERIAL_NONSTANDARD=y
+CONFIG_VIRTIO_CONSOLE=y
 CONFIG_HW_RANDOM=y
 CONFIG_NVRAM=y
 CONFIG_HPET=y
@@ -181,12 +202,7 @@ CONFIG_AGP_AMD64=y
 CONFIG_AGP_INTEL=y
 CONFIG_DRM=y
 CONFIG_DRM_I915=y
-CONFIG_FB_MODE_HELPERS=y
-CONFIG_FB_TILEBLITTING=y
-CONFIG_FB_EFI=y
-CONFIG_LOGO=y
-# CONFIG_LOGO_LINUX_MONO is not set
-# CONFIG_LOGO_LINUX_VGA16 is not set
+CONFIG_DRM_VIRTIO_GPU=y
 CONFIG_SOUND=y
 CONFIG_SND=y
 CONFIG_SND_HRTIMER=y
@@ -219,6 +235,8 @@ CONFIG_USB_STORAGE=y
 CONFIG_RTC_CLASS=y
 # CONFIG_RTC_HCTOSYS is not set
 CONFIG_DMADEVICES=y
+CONFIG_VIRTIO_PCI=y
+CONFIG_VIRTIO_INPUT=y
 CONFIG_EEEPC_LAPTOP=y
 CONFIG_EXT4_FS=y
 CONFIG_EXT4_FS_POSIX_ACL=y
@@ -240,6 +258,7 @@ CONFIG_NFS_FS=y
 CONFIG_NFS_V3_ACL=y
 CONFIG_NFS_V4=y
 CONFIG_ROOT_NFS=y
+CONFIG_9P_FS=y
 CONFIG_NLS_DEFAULT="utf8"
 CONFIG_NLS_CODEPAGE_437=y
 CONFIG_NLS_ASCII=y
@@ -251,15 +270,14 @@ CONFIG_SECURITY_SELINUX=y
 CONFIG_SECURITY_SELINUX_BOOTPARAM=y
 CONFIG_SECURITY_SELINUX_DISABLE=y
 CONFIG_PRINTK_TIME=y
+CONFIG_FRAME_WARN=1024
 CONFIG_MAGIC_SYSRQ=y
 CONFIG_DEBUG_KERNEL=y
 CONFIG_DEBUG_STACK_USAGE=y
-CONFIG_DEBUG_STACKOVERFLOW=y
 # CONFIG_SCHED_DEBUG is not set
 CONFIG_SCHEDSTATS=y
 CONFIG_BLK_DEV_IO_TRACE=y
 CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
 CONFIG_EARLY_PRINTK_DBGP=y
 CONFIG_DEBUG_BOOT_PARAMS=y
-CONFIG_KALLSYMS_ALL=y
-CONFIG_WERROR=y
+CONFIG_UNWINDER_FRAME_POINTER=y
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index 38c52e4eb30b..69784505a7a8 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -1,3 +1,4 @@
+CONFIG_WERROR=y
 CONFIG_SYSVIPC=y
 CONFIG_POSIX_MQUEUE=y
 CONFIG_AUDIT=y
@@ -11,14 +12,25 @@ CONFIG_TASK_XACCT=y
 CONFIG_TASK_IO_ACCOUNTING=y
 CONFIG_LOG_BUF_SHIFT=18
 CONFIG_CGROUPS=y
+CONFIG_BLK_CGROUP=y
 CONFIG_CGROUP_SCHED=y
+CONFIG_CGROUP_PIDS=y
+CONFIG_CGROUP_RDMA=y
 CONFIG_CGROUP_FREEZER=y
+CONFIG_CGROUP_HUGETLB=y
 CONFIG_CPUSETS=y
+CONFIG_CGROUP_DEVICE=y
 CONFIG_CGROUP_CPUACCT=y
+CONFIG_CGROUP_PERF=y
+CONFIG_CGROUP_MISC=y
+CONFIG_CGROUP_DEBUG=y
 CONFIG_BLK_DEV_INITRD=y
+CONFIG_KALLSYMS_ALL=y
 # CONFIG_COMPAT_BRK is not set
 CONFIG_PROFILING=y
 CONFIG_SMP=y
+CONFIG_HYPERVISOR_GUEST=y
+CONFIG_PARAVIRT=y
 CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
 CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
@@ -41,12 +53,14 @@ CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_X86_ACPI_CPUFREQ=y
 CONFIG_IA32_EMULATION=y
-CONFIG_EFI_VARS=y
 CONFIG_KPROBES=y
 CONFIG_JUMP_LABEL=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 CONFIG_MODULE_FORCE_UNLOAD=y
+CONFIG_BLK_CGROUP_IOLATENCY=y
+CONFIG_BLK_CGROUP_IOCOST=y
+CONFIG_BLK_CGROUP_IOPRIO=y
 CONFIG_BINFMT_MISC=y
 CONFIG_NET=y
 CONFIG_PACKET=y
@@ -101,12 +115,16 @@ CONFIG_IP6_NF_FILTER=y
 CONFIG_IP6_NF_TARGET_REJECT=y
 CONFIG_IP6_NF_MANGLE=y
 CONFIG_NET_SCHED=y
+CONFIG_NET_CLS_CGROUP=y
 CONFIG_NET_EMATCH=y
 CONFIG_NET_CLS_ACT=y
+CONFIG_CGROUP_NET_PRIO=y
 CONFIG_CFG80211=y
 CONFIG_MAC80211=y
 CONFIG_MAC80211_LEDS=y
 CONFIG_RFKILL=y
+CONFIG_NET_9P=y
+CONFIG_NET_9P_VIRTIO=y
 CONFIG_PCI=y
 CONFIG_PCIEPORTBUS=y
 CONFIG_HOTPLUG_PCI=y
@@ -116,13 +134,15 @@ CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_DEBUG_DEVRES=y
 CONFIG_CONNECTOR=y
+CONFIG_EFI_VARS=y
 CONFIG_BLK_DEV_LOOP=y
+CONFIG_VIRTIO_BLK=y
 CONFIG_BLK_DEV_SD=y
 CONFIG_BLK_DEV_SR=y
 CONFIG_CHR_DEV_SG=y
 CONFIG_SCSI_CONSTANTS=y
 CONFIG_SCSI_SPI_ATTRS=y
-# CONFIG_SCSI_LOWLEVEL is not set
+CONFIG_SCSI_VIRTIO=y
 CONFIG_ATA=y
 CONFIG_SATA_AHCI=y
 CONFIG_ATA_PIIX=y
@@ -138,6 +158,7 @@ CONFIG_MACINTOSH_DRIVERS=y
 CONFIG_MAC_EMUMOUSEBTN=y
 CONFIG_NETDEVICES=y
 CONFIG_NETCONSOLE=y
+CONFIG_VIRTIO_NET=y
 CONFIG_TIGON3=y
 CONFIG_NET_TULIP=y
 CONFIG_E100=y
@@ -162,6 +183,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
 CONFIG_SERIAL_8250_DETECT_IRQ=y
 CONFIG_SERIAL_8250_RSA=y
 CONFIG_SERIAL_NONSTANDARD=y
+CONFIG_VIRTIO_CONSOLE=y
 CONFIG_HW_RANDOM=y
 # CONFIG_HW_RANDOM_INTEL is not set
 # CONFIG_HW_RANDOM_AMD is not set
@@ -175,12 +197,7 @@ CONFIG_AGP_AMD64=y
 CONFIG_AGP_INTEL=y
 CONFIG_DRM=y
 CONFIG_DRM_I915=y
-CONFIG_FB_MODE_HELPERS=y
-CONFIG_FB_TILEBLITTING=y
-CONFIG_FB_EFI=y
-CONFIG_LOGO=y
-# CONFIG_LOGO_LINUX_MONO is not set
-# CONFIG_LOGO_LINUX_VGA16 is not set
+CONFIG_DRM_VIRTIO_GPU=y
 CONFIG_SOUND=y
 CONFIG_SND=y
 CONFIG_SND_HRTIMER=y
@@ -213,6 +230,8 @@ CONFIG_USB_STORAGE=y
 CONFIG_RTC_CLASS=y
 # CONFIG_RTC_HCTOSYS is not set
 CONFIG_DMADEVICES=y
+CONFIG_VIRTIO_PCI=y
+CONFIG_VIRTIO_INPUT=y
 CONFIG_EEEPC_LAPTOP=y
 CONFIG_AMD_IOMMU=y
 CONFIG_INTEL_IOMMU=y
@@ -237,6 +256,7 @@ CONFIG_NFS_FS=y
 CONFIG_NFS_V3_ACL=y
 CONFIG_NFS_V4=y
 CONFIG_ROOT_NFS=y
+CONFIG_9P_FS=y
 CONFIG_NLS_DEFAULT="utf8"
 CONFIG_NLS_CODEPAGE_437=y
 CONFIG_NLS_ASCII=y
@@ -257,5 +277,3 @@ CONFIG_BLK_DEV_IO_TRACE=y
 CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
 CONFIG_EARLY_PRINTK_DBGP=y
 CONFIG_DEBUG_BOOT_PARAMS=y
-CONFIG_KALLSYMS_ALL=y
-CONFIG_WERROR=y

RE: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[David Laight]

From: Ingo Molnar
> Sent: 24 March 2022 08:31
...
> But beyond the WERROR oversight, bitrot has gotten to defconfig a bit - see
> attached patch. My test scripts usually enable these - but it's better to
> formalize it.

Yes, it would be nice if it booted on a 'typical' PC system.
These are a lot less variable than they used to be.

But there are a few ethernet drivers and things like USB disk
support that are somewhat useful.

Typical 'distro' kernels contain a lot of things most
people don't need - especially a lot of modules.
It'd be nice to have somewhere else to start from.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Nathan Chancellor]

Hi Ingo,

On Thu, Mar 24, 2022 at 09:31:13AM +0100, Ingo Molnar wrote:

<snip>

> ================>
> 
> From: Ingo Molnar <mingo@kernel.org>
> Date: Thu, 24 Mar 2022 09:18:36 +0100
> Subject: [PATCH] x86/config: Make the x86 defconfigs a bit more usable
> 
>  - Use 'make savedefconfig' to refresh & regenerate the files
>  - Add in KVM boot enablers
>  - Enable the cgroup features most distros rely on
> 
> Signed-off-by: Ingo Molnar <mingo@kernel.org>

This should be super helpful, especially for the "reference point and
minimal test requirements" that you mentioned above, along with being
able to boot more distributions in QEMU without fiddling with
kvm_guest.config and menuconfig.

> ---
>  arch/x86/configs/i386_defconfig   | 50 ++++++++++++++++++++++++++-------------
>  arch/x86/configs/x86_64_defconfig | 38 +++++++++++++++++++++--------
>  2 files changed, 62 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
> index 4b2085243aa3..fde771df72cc 100644
> --- a/arch/x86/configs/i386_defconfig
> +++ b/arch/x86/configs/i386_defconfig
> @@ -1,5 +1,7 @@
> +CONFIG_WERROR=y
>  CONFIG_SYSVIPC=y
>  CONFIG_POSIX_MQUEUE=y
> +CONFIG_USELIB=y
>  CONFIG_AUDIT=y
>  CONFIG_NO_HZ=y
>  CONFIG_HIGH_RES_TIMERS=y
> @@ -11,23 +13,30 @@ CONFIG_TASK_XACCT=y
>  CONFIG_TASK_IO_ACCOUNTING=y
>  CONFIG_LOG_BUF_SHIFT=18
>  CONFIG_CGROUPS=y
> +CONFIG_BLK_CGROUP=y
>  CONFIG_CGROUP_SCHED=y
> +CONFIG_CGROUP_PIDS=y
> +CONFIG_CGROUP_RDMA=y
>  CONFIG_CGROUP_FREEZER=y
> +CONFIG_CGROUP_HUGETLB=y
>  CONFIG_CPUSETS=y
> +CONFIG_CGROUP_DEVICE=y
>  CONFIG_CGROUP_CPUACCT=y
> +CONFIG_CGROUP_PERF=y
> +CONFIG_CGROUP_MISC=y
> +CONFIG_CGROUP_DEBUG=y
>  CONFIG_BLK_DEV_INITRD=y
> +CONFIG_KALLSYMS_ALL=y
>  # CONFIG_COMPAT_BRK is not set
>  CONFIG_PROFILING=y
> -# CONFIG_64BIT is not set

I don't think this is right, this is repeating the problem that was
fixed by commit 76366050eb1b ("x86/defconfigs: Explicitly unset
CONFIG_64BIT in i386_defconfig").

$ make ARCH=i386 savedefconfig

?

>  CONFIG_SMP=y
> -CONFIG_X86_GENERIC=y
> -CONFIG_HPET_TIMER=y
> +CONFIG_HYPERVISOR_GUEST=y
> +CONFIG_PARAVIRT=y
> +CONFIG_NR_CPUS=8
>  CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
> -CONFIG_X86_REBOOTFIXUPS=y
>  CONFIG_MICROCODE_AMD=y
>  CONFIG_X86_MSR=y
>  CONFIG_X86_CPUID=y
> -CONFIG_HIGHPTE=y
>  CONFIG_X86_CHECK_BIOS_CORRUPTION=y
>  # CONFIG_MTRR_SANITIZER is not set
>  CONFIG_EFI=y
> @@ -43,12 +52,15 @@ CONFIG_ACPI_BGRT=y
>  CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE=y
>  CONFIG_CPU_FREQ_GOV_ONDEMAND=y
>  CONFIG_X86_ACPI_CPUFREQ=y
> -CONFIG_EFI_VARS=y
>  CONFIG_KPROBES=y
>  CONFIG_JUMP_LABEL=y
> +CONFIG_COMPAT_32BIT_TIME=y
>  CONFIG_MODULES=y
>  CONFIG_MODULE_UNLOAD=y
>  CONFIG_MODULE_FORCE_UNLOAD=y
> +CONFIG_BLK_CGROUP_IOLATENCY=y
> +CONFIG_BLK_CGROUP_IOCOST=y
> +CONFIG_BLK_CGROUP_IOPRIO=y
>  CONFIG_BINFMT_MISC=y
>  CONFIG_NET=y
>  CONFIG_PACKET=y
> @@ -103,12 +115,16 @@ CONFIG_IP6_NF_FILTER=y
>  CONFIG_IP6_NF_TARGET_REJECT=y
>  CONFIG_IP6_NF_MANGLE=y
>  CONFIG_NET_SCHED=y
> +CONFIG_NET_CLS_CGROUP=y
>  CONFIG_NET_EMATCH=y
>  CONFIG_NET_CLS_ACT=y
> +CONFIG_CGROUP_NET_PRIO=y
>  CONFIG_CFG80211=y
>  CONFIG_MAC80211=y
>  CONFIG_MAC80211_LEDS=y
>  CONFIG_RFKILL=y
> +CONFIG_NET_9P=y
> +CONFIG_NET_9P_VIRTIO=y
>  CONFIG_PCI=y
>  CONFIG_PCIEPORTBUS=y
>  CONFIG_PCI_MSI=y
> @@ -119,13 +135,16 @@ CONFIG_DEVTMPFS=y
>  CONFIG_DEVTMPFS_MOUNT=y
>  CONFIG_DEBUG_DEVRES=y
>  CONFIG_CONNECTOR=y
> +CONFIG_EFI_VARS=y
> +CONFIG_EFI_CAPSULE_LOADER=y
>  CONFIG_BLK_DEV_LOOP=y
> +CONFIG_VIRTIO_BLK=y
>  CONFIG_BLK_DEV_SD=y
>  CONFIG_BLK_DEV_SR=y
>  CONFIG_CHR_DEV_SG=y
>  CONFIG_SCSI_CONSTANTS=y
>  CONFIG_SCSI_SPI_ATTRS=y
> -# CONFIG_SCSI_LOWLEVEL is not set
> +CONFIG_SCSI_VIRTIO=y
>  CONFIG_ATA=y
>  CONFIG_SATA_AHCI=y
>  CONFIG_ATA_PIIX=y
> @@ -143,6 +162,7 @@ CONFIG_MACINTOSH_DRIVERS=y
>  CONFIG_MAC_EMUMOUSEBTN=y
>  CONFIG_NETDEVICES=y
>  CONFIG_NETCONSOLE=y
> +CONFIG_VIRTIO_NET=y
>  CONFIG_BNX2=y
>  CONFIG_TIGON3=y
>  CONFIG_NET_TULIP=y
> @@ -170,6 +190,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
>  CONFIG_SERIAL_8250_DETECT_IRQ=y
>  CONFIG_SERIAL_8250_RSA=y
>  CONFIG_SERIAL_NONSTANDARD=y
> +CONFIG_VIRTIO_CONSOLE=y
>  CONFIG_HW_RANDOM=y
>  CONFIG_NVRAM=y
>  CONFIG_HPET=y
> @@ -181,12 +202,7 @@ CONFIG_AGP_AMD64=y
>  CONFIG_AGP_INTEL=y
>  CONFIG_DRM=y
>  CONFIG_DRM_I915=y
> -CONFIG_FB_MODE_HELPERS=y
> -CONFIG_FB_TILEBLITTING=y
> -CONFIG_FB_EFI=y
> -CONFIG_LOGO=y
> -# CONFIG_LOGO_LINUX_MONO is not set
> -# CONFIG_LOGO_LINUX_VGA16 is not set
> +CONFIG_DRM_VIRTIO_GPU=y
>  CONFIG_SOUND=y
>  CONFIG_SND=y
>  CONFIG_SND_HRTIMER=y
> @@ -219,6 +235,8 @@ CONFIG_USB_STORAGE=y
>  CONFIG_RTC_CLASS=y
>  # CONFIG_RTC_HCTOSYS is not set
>  CONFIG_DMADEVICES=y
> +CONFIG_VIRTIO_PCI=y
> +CONFIG_VIRTIO_INPUT=y
>  CONFIG_EEEPC_LAPTOP=y
>  CONFIG_EXT4_FS=y
>  CONFIG_EXT4_FS_POSIX_ACL=y
> @@ -240,6 +258,7 @@ CONFIG_NFS_FS=y
>  CONFIG_NFS_V3_ACL=y
>  CONFIG_NFS_V4=y
>  CONFIG_ROOT_NFS=y
> +CONFIG_9P_FS=y
>  CONFIG_NLS_DEFAULT="utf8"
>  CONFIG_NLS_CODEPAGE_437=y
>  CONFIG_NLS_ASCII=y
> @@ -251,15 +270,14 @@ CONFIG_SECURITY_SELINUX=y
>  CONFIG_SECURITY_SELINUX_BOOTPARAM=y
>  CONFIG_SECURITY_SELINUX_DISABLE=y
>  CONFIG_PRINTK_TIME=y
> +CONFIG_FRAME_WARN=1024
>  CONFIG_MAGIC_SYSRQ=y
>  CONFIG_DEBUG_KERNEL=y
>  CONFIG_DEBUG_STACK_USAGE=y
> -CONFIG_DEBUG_STACKOVERFLOW=y
>  # CONFIG_SCHED_DEBUG is not set
>  CONFIG_SCHEDSTATS=y
>  CONFIG_BLK_DEV_IO_TRACE=y
>  CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
>  CONFIG_EARLY_PRINTK_DBGP=y
>  CONFIG_DEBUG_BOOT_PARAMS=y
> -CONFIG_KALLSYMS_ALL=y
> -CONFIG_WERROR=y
> +CONFIG_UNWINDER_FRAME_POINTER=y
> diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
> index 38c52e4eb30b..69784505a7a8 100644
> --- a/arch/x86/configs/x86_64_defconfig
> +++ b/arch/x86/configs/x86_64_defconfig
> @@ -1,3 +1,4 @@
> +CONFIG_WERROR=y
>  CONFIG_SYSVIPC=y
>  CONFIG_POSIX_MQUEUE=y
>  CONFIG_AUDIT=y
> @@ -11,14 +12,25 @@ CONFIG_TASK_XACCT=y
>  CONFIG_TASK_IO_ACCOUNTING=y
>  CONFIG_LOG_BUF_SHIFT=18
>  CONFIG_CGROUPS=y
> +CONFIG_BLK_CGROUP=y
>  CONFIG_CGROUP_SCHED=y
> +CONFIG_CGROUP_PIDS=y
> +CONFIG_CGROUP_RDMA=y
>  CONFIG_CGROUP_FREEZER=y
> +CONFIG_CGROUP_HUGETLB=y
>  CONFIG_CPUSETS=y
> +CONFIG_CGROUP_DEVICE=y
>  CONFIG_CGROUP_CPUACCT=y
> +CONFIG_CGROUP_PERF=y
> +CONFIG_CGROUP_MISC=y
> +CONFIG_CGROUP_DEBUG=y
>  CONFIG_BLK_DEV_INITRD=y
> +CONFIG_KALLSYMS_ALL=y
>  # CONFIG_COMPAT_BRK is not set
>  CONFIG_PROFILING=y
>  CONFIG_SMP=y
> +CONFIG_HYPERVISOR_GUEST=y
> +CONFIG_PARAVIRT=y
>  CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
>  CONFIG_MICROCODE_AMD=y
>  CONFIG_X86_MSR=y
> @@ -41,12 +53,14 @@ CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE=y
>  CONFIG_CPU_FREQ_GOV_ONDEMAND=y
>  CONFIG_X86_ACPI_CPUFREQ=y
>  CONFIG_IA32_EMULATION=y
> -CONFIG_EFI_VARS=y
>  CONFIG_KPROBES=y
>  CONFIG_JUMP_LABEL=y
>  CONFIG_MODULES=y
>  CONFIG_MODULE_UNLOAD=y
>  CONFIG_MODULE_FORCE_UNLOAD=y
> +CONFIG_BLK_CGROUP_IOLATENCY=y
> +CONFIG_BLK_CGROUP_IOCOST=y
> +CONFIG_BLK_CGROUP_IOPRIO=y
>  CONFIG_BINFMT_MISC=y
>  CONFIG_NET=y
>  CONFIG_PACKET=y
> @@ -101,12 +115,16 @@ CONFIG_IP6_NF_FILTER=y
>  CONFIG_IP6_NF_TARGET_REJECT=y
>  CONFIG_IP6_NF_MANGLE=y
>  CONFIG_NET_SCHED=y
> +CONFIG_NET_CLS_CGROUP=y
>  CONFIG_NET_EMATCH=y
>  CONFIG_NET_CLS_ACT=y
> +CONFIG_CGROUP_NET_PRIO=y
>  CONFIG_CFG80211=y
>  CONFIG_MAC80211=y
>  CONFIG_MAC80211_LEDS=y
>  CONFIG_RFKILL=y
> +CONFIG_NET_9P=y
> +CONFIG_NET_9P_VIRTIO=y
>  CONFIG_PCI=y
>  CONFIG_PCIEPORTBUS=y
>  CONFIG_HOTPLUG_PCI=y
> @@ -116,13 +134,15 @@ CONFIG_DEVTMPFS=y
>  CONFIG_DEVTMPFS_MOUNT=y
>  CONFIG_DEBUG_DEVRES=y
>  CONFIG_CONNECTOR=y
> +CONFIG_EFI_VARS=y
>  CONFIG_BLK_DEV_LOOP=y
> +CONFIG_VIRTIO_BLK=y
>  CONFIG_BLK_DEV_SD=y
>  CONFIG_BLK_DEV_SR=y
>  CONFIG_CHR_DEV_SG=y
>  CONFIG_SCSI_CONSTANTS=y
>  CONFIG_SCSI_SPI_ATTRS=y
> -# CONFIG_SCSI_LOWLEVEL is not set
> +CONFIG_SCSI_VIRTIO=y
>  CONFIG_ATA=y
>  CONFIG_SATA_AHCI=y
>  CONFIG_ATA_PIIX=y
> @@ -138,6 +158,7 @@ CONFIG_MACINTOSH_DRIVERS=y
>  CONFIG_MAC_EMUMOUSEBTN=y
>  CONFIG_NETDEVICES=y
>  CONFIG_NETCONSOLE=y
> +CONFIG_VIRTIO_NET=y
>  CONFIG_TIGON3=y
>  CONFIG_NET_TULIP=y
>  CONFIG_E100=y
> @@ -162,6 +183,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
>  CONFIG_SERIAL_8250_DETECT_IRQ=y
>  CONFIG_SERIAL_8250_RSA=y
>  CONFIG_SERIAL_NONSTANDARD=y
> +CONFIG_VIRTIO_CONSOLE=y
>  CONFIG_HW_RANDOM=y
>  # CONFIG_HW_RANDOM_INTEL is not set
>  # CONFIG_HW_RANDOM_AMD is not set
> @@ -175,12 +197,7 @@ CONFIG_AGP_AMD64=y
>  CONFIG_AGP_INTEL=y
>  CONFIG_DRM=y
>  CONFIG_DRM_I915=y
> -CONFIG_FB_MODE_HELPERS=y
> -CONFIG_FB_TILEBLITTING=y
> -CONFIG_FB_EFI=y
> -CONFIG_LOGO=y
> -# CONFIG_LOGO_LINUX_MONO is not set
> -# CONFIG_LOGO_LINUX_VGA16 is not set
> +CONFIG_DRM_VIRTIO_GPU=y
>  CONFIG_SOUND=y
>  CONFIG_SND=y
>  CONFIG_SND_HRTIMER=y
> @@ -213,6 +230,8 @@ CONFIG_USB_STORAGE=y
>  CONFIG_RTC_CLASS=y
>  # CONFIG_RTC_HCTOSYS is not set
>  CONFIG_DMADEVICES=y
> +CONFIG_VIRTIO_PCI=y
> +CONFIG_VIRTIO_INPUT=y
>  CONFIG_EEEPC_LAPTOP=y
>  CONFIG_AMD_IOMMU=y
>  CONFIG_INTEL_IOMMU=y
> @@ -237,6 +256,7 @@ CONFIG_NFS_FS=y
>  CONFIG_NFS_V3_ACL=y
>  CONFIG_NFS_V4=y
>  CONFIG_ROOT_NFS=y
> +CONFIG_9P_FS=y
>  CONFIG_NLS_DEFAULT="utf8"
>  CONFIG_NLS_CODEPAGE_437=y
>  CONFIG_NLS_ASCII=y
> @@ -257,5 +277,3 @@ CONFIG_BLK_DEV_IO_TRACE=y
>  CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
>  CONFIG_EARLY_PRINTK_DBGP=y
>  CONFIG_DEBUG_BOOT_PARAMS=y
> -CONFIG_KALLSYMS_ALL=y
> -CONFIG_WERROR=y
> 

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Andy Shevchenko]

On Thu, Mar 24, 2022 at 08:47:27AM -0700, Nathan Chancellor wrote:
> On Thu, Mar 24, 2022 at 09:31:13AM +0100, Ingo Molnar wrote:

> > --- a/arch/x86/configs/i386_defconfig
> > +++ b/arch/x86/configs/i386_defconfig

> > -# CONFIG_64BIT is not set
> 
> I don't think this is right, this is repeating the problem that was
> fixed by commit 76366050eb1b ("x86/defconfigs: Explicitly unset
> CONFIG_64BIT in i386_defconfig").
> 
> $ make ARCH=i386 savedefconfig
> 
> ?

+1 here, there are still branches in use that have i386 defconfig as
as base and making this problem again is not okay.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Ingo Molnar]

* Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Thu, Mar 24, 2022 at 08:47:27AM -0700, Nathan Chancellor wrote:
> > On Thu, Mar 24, 2022 at 09:31:13AM +0100, Ingo Molnar wrote:
> 
> > > --- a/arch/x86/configs/i386_defconfig
> > > +++ b/arch/x86/configs/i386_defconfig
> 
> > > -# CONFIG_64BIT is not set
> > 
> > I don't think this is right, this is repeating the problem that was
> > fixed by commit 76366050eb1b ("x86/defconfigs: Explicitly unset
> > CONFIG_64BIT in i386_defconfig").
> > 
> > $ make ARCH=i386 savedefconfig
> > 
> > ?
> 
> +1 here, there are still branches in use that have i386 defconfig as
> as base and making this problem again is not okay.

Absolutely - fixed in the latest version.

Thanks,

	Ingo

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Ingo Molnar]

* Nathan Chancellor <nathan@kernel.org> wrote:

> Hi Ingo,
> 
> On Thu, Mar 24, 2022 at 09:31:13AM +0100, Ingo Molnar wrote:
> 
> <snip>
> 
> > ================>
> > 
> > From: Ingo Molnar <mingo@kernel.org>
> > Date: Thu, 24 Mar 2022 09:18:36 +0100
> > Subject: [PATCH] x86/config: Make the x86 defconfigs a bit more usable
> > 
> >  - Use 'make savedefconfig' to refresh & regenerate the files
> >  - Add in KVM boot enablers
> >  - Enable the cgroup features most distros rely on
> > 
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> 
> This should be super helpful, especially for the "reference point and
> minimal test requirements" that you mentioned above, along with being
> able to boot more distributions in QEMU without fiddling with
> kvm_guest.config and menuconfig.

Thanks, I've added your Acked-by to the (fixed...) version of this commit.

> >  # CONFIG_COMPAT_BRK is not set
> >  CONFIG_PROFILING=y
> > -# CONFIG_64BIT is not set
> 
> I don't think this is right, this is repeating the problem that was fixed 
> by commit 76366050eb1b ("x86/defconfigs: Explicitly unset CONFIG_64BIT in 
> i386_defconfig").
> 
> $ make ARCH=i386 savedefconfig
> 
> ?

Yeah, good catch! ...

I use defconfigs by explicitly adding in the architecture:

  kepler:~/tip> make ARCH=i386 defconfig

  kepler:~/tip> kconfig-arch
  i386

... so never I noticed this bug.

I fixed this in the latest version (attached).

Arguably 'make ARCH=i386 savedefconfig' should preserve this, so that 
refreshing defconfigs on bi-arch architectures is idempotent, but that's no 
excuse to regress the existing defconfig behavior.

Thanks,

	Ingo

========================>
From: Ingo Molnar <mingo@kernel.org>
Date: Thu, 24 Mar 2022 09:31:13 +0100
Subject: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

 - Use 'make savedefconfig' to refresh & regenerate the files
 - Add in KVM boot enablers
 - Enable the cgroup features most distros rely on

[ fix bug found by Nathan Chancellor ]

Acked-by: Nathan Chancellor <nathan@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/YjwsUT/6PkRPjnHE@gmail.com
---
 arch/x86/configs/i386_defconfig   | 51 +++++++++++++++++++++++++++------------
 arch/x86/configs/x86_64_defconfig | 38 +++++++++++++++++++++--------
 2 files changed, 63 insertions(+), 26 deletions(-)

diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
index 4b2085243aa3..98a4852ed6a0 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -1,5 +1,7 @@
+CONFIG_WERROR=y
 CONFIG_SYSVIPC=y
 CONFIG_POSIX_MQUEUE=y
+CONFIG_USELIB=y
 CONFIG_AUDIT=y
 CONFIG_NO_HZ=y
 CONFIG_HIGH_RES_TIMERS=y
@@ -11,23 +13,30 @@ CONFIG_TASK_XACCT=y
 CONFIG_TASK_IO_ACCOUNTING=y
 CONFIG_LOG_BUF_SHIFT=18
 CONFIG_CGROUPS=y
+CONFIG_BLK_CGROUP=y
 CONFIG_CGROUP_SCHED=y
+CONFIG_CGROUP_PIDS=y
+CONFIG_CGROUP_RDMA=y
 CONFIG_CGROUP_FREEZER=y
+CONFIG_CGROUP_HUGETLB=y
 CONFIG_CPUSETS=y
+CONFIG_CGROUP_DEVICE=y
 CONFIG_CGROUP_CPUACCT=y
+CONFIG_CGROUP_PERF=y
+CONFIG_CGROUP_MISC=y
+CONFIG_CGROUP_DEBUG=y
 CONFIG_BLK_DEV_INITRD=y
+CONFIG_KALLSYMS_ALL=y
 # CONFIG_COMPAT_BRK is not set
 CONFIG_PROFILING=y
-# CONFIG_64BIT is not set
 CONFIG_SMP=y
-CONFIG_X86_GENERIC=y
-CONFIG_HPET_TIMER=y
+CONFIG_HYPERVISOR_GUEST=y
+CONFIG_PARAVIRT=y
+CONFIG_NR_CPUS=8
 CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
-CONFIG_X86_REBOOTFIXUPS=y
 CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
 CONFIG_X86_CPUID=y
-CONFIG_HIGHPTE=y
 CONFIG_X86_CHECK_BIOS_CORRUPTION=y
 # CONFIG_MTRR_SANITIZER is not set
 CONFIG_EFI=y
@@ -43,12 +52,15 @@ CONFIG_ACPI_BGRT=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_X86_ACPI_CPUFREQ=y
-CONFIG_EFI_VARS=y
 CONFIG_KPROBES=y
 CONFIG_JUMP_LABEL=y
+CONFIG_COMPAT_32BIT_TIME=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 CONFIG_MODULE_FORCE_UNLOAD=y
+CONFIG_BLK_CGROUP_IOLATENCY=y
+CONFIG_BLK_CGROUP_IOCOST=y
+CONFIG_BLK_CGROUP_IOPRIO=y
 CONFIG_BINFMT_MISC=y
 CONFIG_NET=y
 CONFIG_PACKET=y
@@ -103,12 +115,16 @@ CONFIG_IP6_NF_FILTER=y
 CONFIG_IP6_NF_TARGET_REJECT=y
 CONFIG_IP6_NF_MANGLE=y
 CONFIG_NET_SCHED=y
+CONFIG_NET_CLS_CGROUP=y
 CONFIG_NET_EMATCH=y
 CONFIG_NET_CLS_ACT=y
+CONFIG_CGROUP_NET_PRIO=y
 CONFIG_CFG80211=y
 CONFIG_MAC80211=y
 CONFIG_MAC80211_LEDS=y
 CONFIG_RFKILL=y
+CONFIG_NET_9P=y
+CONFIG_NET_9P_VIRTIO=y
 CONFIG_PCI=y
 CONFIG_PCIEPORTBUS=y
 CONFIG_PCI_MSI=y
@@ -119,13 +135,16 @@ CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_DEBUG_DEVRES=y
 CONFIG_CONNECTOR=y
+CONFIG_EFI_VARS=y
+CONFIG_EFI_CAPSULE_LOADER=y
 CONFIG_BLK_DEV_LOOP=y
+CONFIG_VIRTIO_BLK=y
 CONFIG_BLK_DEV_SD=y
 CONFIG_BLK_DEV_SR=y
 CONFIG_CHR_DEV_SG=y
 CONFIG_SCSI_CONSTANTS=y
 CONFIG_SCSI_SPI_ATTRS=y
-# CONFIG_SCSI_LOWLEVEL is not set
+CONFIG_SCSI_VIRTIO=y
 CONFIG_ATA=y
 CONFIG_SATA_AHCI=y
 CONFIG_ATA_PIIX=y
@@ -143,6 +162,7 @@ CONFIG_MACINTOSH_DRIVERS=y
 CONFIG_MAC_EMUMOUSEBTN=y
 CONFIG_NETDEVICES=y
 CONFIG_NETCONSOLE=y
+CONFIG_VIRTIO_NET=y
 CONFIG_BNX2=y
 CONFIG_TIGON3=y
 CONFIG_NET_TULIP=y
@@ -170,6 +190,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
 CONFIG_SERIAL_8250_DETECT_IRQ=y
 CONFIG_SERIAL_8250_RSA=y
 CONFIG_SERIAL_NONSTANDARD=y
+CONFIG_VIRTIO_CONSOLE=y
 CONFIG_HW_RANDOM=y
 CONFIG_NVRAM=y
 CONFIG_HPET=y
@@ -181,12 +202,7 @@ CONFIG_AGP_AMD64=y
 CONFIG_AGP_INTEL=y
 CONFIG_DRM=y
 CONFIG_DRM_I915=y
-CONFIG_FB_MODE_HELPERS=y
-CONFIG_FB_TILEBLITTING=y
-CONFIG_FB_EFI=y
-CONFIG_LOGO=y
-# CONFIG_LOGO_LINUX_MONO is not set
-# CONFIG_LOGO_LINUX_VGA16 is not set
+CONFIG_DRM_VIRTIO_GPU=y
 CONFIG_SOUND=y
 CONFIG_SND=y
 CONFIG_SND_HRTIMER=y
@@ -219,6 +235,8 @@ CONFIG_USB_STORAGE=y
 CONFIG_RTC_CLASS=y
 # CONFIG_RTC_HCTOSYS is not set
 CONFIG_DMADEVICES=y
+CONFIG_VIRTIO_PCI=y
+CONFIG_VIRTIO_INPUT=y
 CONFIG_EEEPC_LAPTOP=y
 CONFIG_EXT4_FS=y
 CONFIG_EXT4_FS_POSIX_ACL=y
@@ -240,6 +258,7 @@ CONFIG_NFS_FS=y
 CONFIG_NFS_V3_ACL=y
 CONFIG_NFS_V4=y
 CONFIG_ROOT_NFS=y
+CONFIG_9P_FS=y
 CONFIG_NLS_DEFAULT="utf8"
 CONFIG_NLS_CODEPAGE_437=y
 CONFIG_NLS_ASCII=y
@@ -251,15 +270,15 @@ CONFIG_SECURITY_SELINUX=y
 CONFIG_SECURITY_SELINUX_BOOTPARAM=y
 CONFIG_SECURITY_SELINUX_DISABLE=y
 CONFIG_PRINTK_TIME=y
+CONFIG_FRAME_WARN=1024
 CONFIG_MAGIC_SYSRQ=y
 CONFIG_DEBUG_KERNEL=y
 CONFIG_DEBUG_STACK_USAGE=y
-CONFIG_DEBUG_STACKOVERFLOW=y
 # CONFIG_SCHED_DEBUG is not set
 CONFIG_SCHEDSTATS=y
 CONFIG_BLK_DEV_IO_TRACE=y
 CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
 CONFIG_EARLY_PRINTK_DBGP=y
 CONFIG_DEBUG_BOOT_PARAMS=y
-CONFIG_KALLSYMS_ALL=y
-CONFIG_WERROR=y
+CONFIG_UNWINDER_FRAME_POINTER=y
+# CONFIG_64BIT is not set
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index 38c52e4eb30b..69784505a7a8 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -1,3 +1,4 @@
+CONFIG_WERROR=y
 CONFIG_SYSVIPC=y
 CONFIG_POSIX_MQUEUE=y
 CONFIG_AUDIT=y
@@ -11,14 +12,25 @@ CONFIG_TASK_XACCT=y
 CONFIG_TASK_IO_ACCOUNTING=y
 CONFIG_LOG_BUF_SHIFT=18
 CONFIG_CGROUPS=y
+CONFIG_BLK_CGROUP=y
 CONFIG_CGROUP_SCHED=y
+CONFIG_CGROUP_PIDS=y
+CONFIG_CGROUP_RDMA=y
 CONFIG_CGROUP_FREEZER=y
+CONFIG_CGROUP_HUGETLB=y
 CONFIG_CPUSETS=y
+CONFIG_CGROUP_DEVICE=y
 CONFIG_CGROUP_CPUACCT=y
+CONFIG_CGROUP_PERF=y
+CONFIG_CGROUP_MISC=y
+CONFIG_CGROUP_DEBUG=y
 CONFIG_BLK_DEV_INITRD=y
+CONFIG_KALLSYMS_ALL=y
 # CONFIG_COMPAT_BRK is not set
 CONFIG_PROFILING=y
 CONFIG_SMP=y
+CONFIG_HYPERVISOR_GUEST=y
+CONFIG_PARAVIRT=y
 CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
 CONFIG_MICROCODE_AMD=y
 CONFIG_X86_MSR=y
@@ -41,12 +53,14 @@ CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE=y
 CONFIG_CPU_FREQ_GOV_ONDEMAND=y
 CONFIG_X86_ACPI_CPUFREQ=y
 CONFIG_IA32_EMULATION=y
-CONFIG_EFI_VARS=y
 CONFIG_KPROBES=y
 CONFIG_JUMP_LABEL=y
 CONFIG_MODULES=y
 CONFIG_MODULE_UNLOAD=y
 CONFIG_MODULE_FORCE_UNLOAD=y
+CONFIG_BLK_CGROUP_IOLATENCY=y
+CONFIG_BLK_CGROUP_IOCOST=y
+CONFIG_BLK_CGROUP_IOPRIO=y
 CONFIG_BINFMT_MISC=y
 CONFIG_NET=y
 CONFIG_PACKET=y
@@ -101,12 +115,16 @@ CONFIG_IP6_NF_FILTER=y
 CONFIG_IP6_NF_TARGET_REJECT=y
 CONFIG_IP6_NF_MANGLE=y
 CONFIG_NET_SCHED=y
+CONFIG_NET_CLS_CGROUP=y
 CONFIG_NET_EMATCH=y
 CONFIG_NET_CLS_ACT=y
+CONFIG_CGROUP_NET_PRIO=y
 CONFIG_CFG80211=y
 CONFIG_MAC80211=y
 CONFIG_MAC80211_LEDS=y
 CONFIG_RFKILL=y
+CONFIG_NET_9P=y
+CONFIG_NET_9P_VIRTIO=y
 CONFIG_PCI=y
 CONFIG_PCIEPORTBUS=y
 CONFIG_HOTPLUG_PCI=y
@@ -116,13 +134,15 @@ CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_DEBUG_DEVRES=y
 CONFIG_CONNECTOR=y
+CONFIG_EFI_VARS=y
 CONFIG_BLK_DEV_LOOP=y
+CONFIG_VIRTIO_BLK=y
 CONFIG_BLK_DEV_SD=y
 CONFIG_BLK_DEV_SR=y
 CONFIG_CHR_DEV_SG=y
 CONFIG_SCSI_CONSTANTS=y
 CONFIG_SCSI_SPI_ATTRS=y
-# CONFIG_SCSI_LOWLEVEL is not set
+CONFIG_SCSI_VIRTIO=y
 CONFIG_ATA=y
 CONFIG_SATA_AHCI=y
 CONFIG_ATA_PIIX=y
@@ -138,6 +158,7 @@ CONFIG_MACINTOSH_DRIVERS=y
 CONFIG_MAC_EMUMOUSEBTN=y
 CONFIG_NETDEVICES=y
 CONFIG_NETCONSOLE=y
+CONFIG_VIRTIO_NET=y
 CONFIG_TIGON3=y
 CONFIG_NET_TULIP=y
 CONFIG_E100=y
@@ -162,6 +183,7 @@ CONFIG_SERIAL_8250_SHARE_IRQ=y
 CONFIG_SERIAL_8250_DETECT_IRQ=y
 CONFIG_SERIAL_8250_RSA=y
 CONFIG_SERIAL_NONSTANDARD=y
+CONFIG_VIRTIO_CONSOLE=y
 CONFIG_HW_RANDOM=y
 # CONFIG_HW_RANDOM_INTEL is not set
 # CONFIG_HW_RANDOM_AMD is not set
@@ -175,12 +197,7 @@ CONFIG_AGP_AMD64=y
 CONFIG_AGP_INTEL=y
 CONFIG_DRM=y
 CONFIG_DRM_I915=y
-CONFIG_FB_MODE_HELPERS=y
-CONFIG_FB_TILEBLITTING=y
-CONFIG_FB_EFI=y
-CONFIG_LOGO=y
-# CONFIG_LOGO_LINUX_MONO is not set
-# CONFIG_LOGO_LINUX_VGA16 is not set
+CONFIG_DRM_VIRTIO_GPU=y
 CONFIG_SOUND=y
 CONFIG_SND=y
 CONFIG_SND_HRTIMER=y
@@ -213,6 +230,8 @@ CONFIG_USB_STORAGE=y
 CONFIG_RTC_CLASS=y
 # CONFIG_RTC_HCTOSYS is not set
 CONFIG_DMADEVICES=y
+CONFIG_VIRTIO_PCI=y
+CONFIG_VIRTIO_INPUT=y
 CONFIG_EEEPC_LAPTOP=y
 CONFIG_AMD_IOMMU=y
 CONFIG_INTEL_IOMMU=y
@@ -237,6 +256,7 @@ CONFIG_NFS_FS=y
 CONFIG_NFS_V3_ACL=y
 CONFIG_NFS_V4=y
 CONFIG_ROOT_NFS=y
+CONFIG_9P_FS=y
 CONFIG_NLS_DEFAULT="utf8"
 CONFIG_NLS_CODEPAGE_437=y
 CONFIG_NLS_ASCII=y
@@ -257,5 +277,3 @@ CONFIG_BLK_DEV_IO_TRACE=y
 CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
 CONFIG_EARLY_PRINTK_DBGP=y
 CONFIG_DEBUG_BOOT_PARAMS=y
-CONFIG_KALLSYMS_ALL=y
-CONFIG_WERROR=y

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Nathan Chancellor]

On Sun, Mar 27, 2022 at 09:03:14PM +0200, Ingo Molnar wrote:
> Yeah, good catch! ...
> 
> I use defconfigs by explicitly adding in the architecture:
> 
>   kepler:~/tip> make ARCH=i386 defconfig
> 
>   kepler:~/tip> kconfig-arch
>   i386
> 
> ... so never I noticed this bug.

Ah, good point!

> I fixed this in the latest version (attached).
> 
> Arguably 'make ARCH=i386 savedefconfig' should preserve this, so that 
> refreshing defconfigs on bi-arch architectures is idempotent, but that's no 
> excuse to regress the existing defconfig behavior.

Hmmm, I thought that it would, but I think the behavior of savedefconfig
is actually correct with regards to how it handles CONFIG_64BIT in the
presence of an explicit ARCH value, based on how CONFIG_64BIT is
defined:

config 64BIT
	bool "64-bit kernel" if "$(ARCH)" = "x86"
	default "$(ARCH)" != "i386"
	help
	  Say yes to build a 64-bit kernel - formerly known as x86_64
	  Say no to build a 32-bit kernel - formerly known as i386

As the default is no when ARCH == i386 and there is no prompt in that
situation, "# CONFIG_64BIT is not set" gets dropped, as that is the
default. Using savedefconfig without the ARCH variable would do the
right thing.

I tried playing around with these Kconfig symbols to see if I could get
something that would work for savedefconfig with or without ARCH, but I
could not really come up with anything. I did not try super hard though,
it might still be possible.

Cheers,
Nathan

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Ingo Molnar]

* Nathan Chancellor <nathan@kernel.org> wrote:

> On Sun, Mar 27, 2022 at 09:03:14PM +0200, Ingo Molnar wrote:
> > Yeah, good catch! ...
> > 
> > I use defconfigs by explicitly adding in the architecture:
> > 
> >   kepler:~/tip> make ARCH=i386 defconfig
> > 
> >   kepler:~/tip> kconfig-arch
> >   i386
> > 
> > ... so never I noticed this bug.
> 
> Ah, good point!
> 
> > I fixed this in the latest version (attached).
> > 
> > Arguably 'make ARCH=i386 savedefconfig' should preserve this, so that 
> > refreshing defconfigs on bi-arch architectures is idempotent, but that's no 
> > excuse to regress the existing defconfig behavior.
> 
> Hmmm, I thought that it would, but I think the behavior of savedefconfig
> is actually correct with regards to how it handles CONFIG_64BIT in the
> presence of an explicit ARCH value, based on how CONFIG_64BIT is
> defined:
> 
> config 64BIT
> 	bool "64-bit kernel" if "$(ARCH)" = "x86"
> 	default "$(ARCH)" != "i386"
> 	help
> 	  Say yes to build a 64-bit kernel - formerly known as x86_64
> 	  Say no to build a 32-bit kernel - formerly known as i386
> 
> As the default is no when ARCH == i386 and there is no prompt in that
> situation, "# CONFIG_64BIT is not set" gets dropped, as that is the
> default. Using savedefconfig without the ARCH variable would do the
> right thing.
> 
> I tried playing around with these Kconfig symbols to see if I could get
> something that would work for savedefconfig with or without ARCH, but I
> could not really come up with anything. I did not try super hard though,
> it might still be possible.

Unfortunately, even without the ARCH=i386 'make savedefconfig' doesn't seem 
to be doing the right thing & is dropping the '# CONFIG_64BIT is not set' 
line:

 kepler:~/tip> make ARCH=i386 defconfig
 *** Default configuration is based on 'i386_defconfig'
 #
 # configuration written to .config
 #
 kepler:~/tip> make savedefconfig
 kepler:~/tip> diff -up arch/x86/configs/i386_defconfig defconfig
 --- arch/x86/configs/i386_defconfig	2022-09-02 10:45:43.117430882 +0200
 +++ defconfig	2022-09-02 10:46:56.663864901 +0200
 @@ -282,4 +282,3 @@ CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
  CONFIG_EARLY_PRINTK_DBGP=y
  CONFIG_DEBUG_BOOT_PARAMS=y
  CONFIG_UNWINDER_FRAME_POINTER=y
 -# CONFIG_64BIT is not set
 kepler:~/tip> 

This is annoying in that every time I modify the i386 defconfig and use 
'make savedefconfig', I have to manually revert that change ...

This reduces the usability of 'make savedefconfig' quite a bit.

Maybe Masahiro-san can tell me how I'm doing this wrong?

Thanks,

	Ingo

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Masahiro Yamada]

On Fri, Sep 2, 2022 at 5:50 PM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Nathan Chancellor <nathan@kernel.org> wrote:
>
> > On Sun, Mar 27, 2022 at 09:03:14PM +0200, Ingo Molnar wrote:
> > > Yeah, good catch! ...
> > >
> > > I use defconfigs by explicitly adding in the architecture:
> > >
> > >   kepler:~/tip> make ARCH=i386 defconfig
> > >
> > >   kepler:~/tip> kconfig-arch
> > >   i386
> > >
> > > ... so never I noticed this bug.
> >
> > Ah, good point!
> >
> > > I fixed this in the latest version (attached).
> > >
> > > Arguably 'make ARCH=i386 savedefconfig' should preserve this, so that
> > > refreshing defconfigs on bi-arch architectures is idempotent, but that's no
> > > excuse to regress the existing defconfig behavior.
> >
> > Hmmm, I thought that it would, but I think the behavior of savedefconfig
> > is actually correct with regards to how it handles CONFIG_64BIT in the
> > presence of an explicit ARCH value, based on how CONFIG_64BIT is
> > defined:
> >
> > config 64BIT
> >       bool "64-bit kernel" if "$(ARCH)" = "x86"
> >       default "$(ARCH)" != "i386"
> >       help
> >         Say yes to build a 64-bit kernel - formerly known as x86_64
> >         Say no to build a 32-bit kernel - formerly known as i386
> >
> > As the default is no when ARCH == i386 and there is no prompt in that
> > situation, "# CONFIG_64BIT is not set" gets dropped, as that is the
> > default. Using savedefconfig without the ARCH variable would do the
> > right thing.
> >
> > I tried playing around with these Kconfig symbols to see if I could get
> > something that would work for savedefconfig with or without ARCH, but I
> > could not really come up with anything. I did not try super hard though,
> > it might still be possible.
>
> Unfortunately, even without the ARCH=i386 'make savedefconfig' doesn't seem
> to be doing the right thing & is dropping the '# CONFIG_64BIT is not set'
> line:


Oh, really?

Without ARCH=i386, it works correctly for me.



masahiro@zoe:~/ref/linux$ make i386_defconfig savedefconfig
#
# No change to .config
#
masahiro@zoe:~/ref/linux$ grep CONFIG_64BIT defconfig
# CONFIG_64BIT is not set




I guess you gave ARCH=i386 somewhere.







>  kepler:~/tip> make ARCH=i386 defconfig
>  *** Default configuration is based on 'i386_defconfig'
>  #
>  # configuration written to .config
>  #
>  kepler:~/tip> make savedefconfig
>  kepler:~/tip> diff -up arch/x86/configs/i386_defconfig defconfig
>  --- arch/x86/configs/i386_defconfig    2022-09-02 10:45:43.117430882 +0200
>  +++ defconfig  2022-09-02 10:46:56.663864901 +0200
>  @@ -282,4 +282,3 @@ CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
>   CONFIG_EARLY_PRINTK_DBGP=y
>   CONFIG_DEBUG_BOOT_PARAMS=y
>   CONFIG_UNWINDER_FRAME_POINTER=y
>  -# CONFIG_64BIT is not set
>  kepler:~/tip>
>
> This is annoying in that every time I modify the i386 defconfig and use
> 'make savedefconfig', I have to manually revert that change ...
>
> This reduces the usability of 'make savedefconfig' quite a bit.
>
> Maybe Masahiro-san can tell me how I'm doing this wrong?
>
> Thanks,
>
>         Ingo



--
Best Regards
Masahiro Yamada

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Ingo Molnar]

* Masahiro Yamada <masahiroy@kernel.org> wrote:

> > Unfortunately, even without the ARCH=i386 'make savedefconfig' doesn't 
> > seem to be doing the right thing & is dropping the '# CONFIG_64BIT is 
> > not set' line:
> 
> 
> Oh, really?
> 
> Without ARCH=i386, it works correctly for me.
> 
> 
> 
> masahiro@zoe:~/ref/linux$ make i386_defconfig savedefconfig
> #
> # No change to .config
> #
> masahiro@zoe:~/ref/linux$ grep CONFIG_64BIT defconfig
> # CONFIG_64BIT is not set

Yeah, so why do these two seemingly identical commands produce two 
different .config's:

  $ make ARCH=i386 defconfig
  *** Default configuration is based on 'i386_defconfig'

  $ make i386_defconfig

?

Basically the canonical way to generate a defconfig is to provide an ARCH, 
then use the common 'defconfig' target:

   make ARCH=i386      defconfig
   make ARCH=x86       defconfig
   make ARCH=arm       defconfig
   make ARCH=arm64     defconfig
   make ARCH=powerpc   defconfig
   make ARCH=s390      defconfig
   make ARCH=sparc     defconfig
   make ARCH=sparc64   defconfig
   ...

This is what my build test scripts are using to quick-test cross-arch 
builds. It's a straightforward way to generate defconfigs without knowing 
the details of the myriads of random defconfig targets some architectures 
have, such as:

  kepler:~/tip> ls arch/arm/configs/
  am200epdkit_defconfig     gemini_defconfig         multi_v5_defconfig    s5pv210_defconfig
  aspeed_g4_defconfig       h3600_defconfig          multi_v7_defconfig    sama5_defconfig
  aspeed_g5_defconfig       h5000_defconfig          mv78xx0_defconfig     sama7_defconfig
  assabet_defconfig         hackkit_defconfig        mvebu_v5_defconfig    shannon_defconfig
  at91_dt_defconfig         hisi_defconfig           mvebu_v7_defconfig    shmobile_defconfig
  axm55xx_defconfig         imxrt_defconfig          mxs_defconfig         simpad_defconfig
  badge4_defconfig          imx_v4_v5_defconfig      neponset_defconfig    socfpga_defconfig
  bcm2835_defconfig         imx_v6_v7_defconfig      netwinder_defconfig   sp7021_defconfig
  cerfcube_defconfig        integrator_defconfig     nhk8815_defconfig     spear13xx_defconfig
  clps711x_defconfig        iop32x_defconfig         omap1_defconfig       spear3xx_defconfig
  cm_x300_defconfig         ixp4xx_defconfig         omap2plus_defconfig   spear6xx_defconfig
  cns3420vb_defconfig       jornada720_defconfig     orion5x_defconfig     spitz_defconfig
  colibri_pxa270_defconfig  keystone_defconfig       oxnas_v6_defconfig    stm32_defconfig
  colibri_pxa300_defconfig  lart_defconfig           palmz72_defconfig     sunxi_defconfig
  collie_defconfig          lpc18xx_defconfig        pcm027_defconfig      tct_hammer_defconfig
  corgi_defconfig           lpc32xx_defconfig        pleb_defconfig        tegra_defconfig
  davinci_all_defconfig     lpd270_defconfig         pxa168_defconfig      trizeps4_defconfig
  dove_defconfig            lubbock_defconfig        pxa255-idp_defconfig  u8500_defconfig
  dram_0x00000000.config    magician_defconfig       pxa3xx_defconfig      versatile_defconfig
  dram_0xc0000000.config    mainstone_defconfig      pxa910_defconfig      vexpress_defconfig
  dram_0xd0000000.config    milbeaut_m10v_defconfig  pxa_defconfig         vf610m4_defconfig
  ep93xx_defconfig          mini2440_defconfig       qcom_defconfig        viper_defconfig
  eseries_pxa_defconfig     mmp2_defconfig           realview_defconfig    vt8500_v6_v7_defconfig
  exynos_defconfig          moxart_defconfig         rpc_defconfig         xcep_defconfig
  ezx_defconfig             mps2_defconfig           s3c2410_defconfig     zeus_defconfig
  footbridge_defconfig      multi_v4t_defconfig      s3c6400_defconfig

But this doesn't seem to be working reliably on i386:

  kepler:~/tip> make i386_defconfig
  #
  # configuration written to .config
  #
  kepler:~/tip> /bin/cp .config .config.i386_defconfig.1

  kepler:~/tip> make ARCH=i386 defconfig
  *** Default configuration is based on 'i386_defconfig'
  #
  # configuration written to .config
  #
  kepler:~/tip> /bin/cp .config .config.i386_defconfig.2

  kepler:~/tip> diff -up .config.i386_defconfig.1 .config.i386_defconfig.2
  --- .config.i386_defconfig.1	2022-09-04 11:34:48.253202205 +0200
  +++ .config.i386_defconfig.2	2022-09-04 11:35:04.268758331 +0200
  @@ -1,6 +1,6 @@
   #
   # Automatically generated file; DO NOT EDIT.
  -# Linux/x86 6.0.0-rc3 Kernel Configuration
  +# Linux/i386 6.0.0-rc3 Kernel Configuration
   #
   CONFIG_CC_VERSION_TEXT="gcc (Ubuntu 12-20220319-1ubuntu1) 12.0.1 20220319 (experimental) [master r12-7719-g8ca61ad148f]"
   CONFIG_CC_IS_GCC=y
  @@ -261,7 +261,6 @@ CONFIG_PROFILING=y
   CONFIG_TRACEPOINTS=y
   # end of General setup
   
  -# CONFIG_64BIT is not set
   CONFIG_X86_32=y
   CONFIG_X86=y
   CONFIG_INSTRUCTION_DECODER=y

Note how in the ARCH=i386 case the build system claims to use i386_defconfig:

  *** Default configuration is based on 'i386_defconfig'

But that doesn't seem to be identical with when we specify i386_defconfig 
directly as a target...

What am I missing?

Thanks,

	Ingo

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Masahiro Yamada]

On Sun, Sep 4, 2022 at 6:48 PM Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> > > Unfortunately, even without the ARCH=i386 'make savedefconfig' doesn't
> > > seem to be doing the right thing & is dropping the '# CONFIG_64BIT is
> > > not set' line:
> >
> >
> > Oh, really?
> >
> > Without ARCH=i386, it works correctly for me.
> >
> >
> >
> > masahiro@zoe:~/ref/linux$ make i386_defconfig savedefconfig
> > #
> > # No change to .config
> > #
> > masahiro@zoe:~/ref/linux$ grep CONFIG_64BIT defconfig
> > # CONFIG_64BIT is not set
>
> Yeah, so why do these two seemingly identical commands produce two
> different .config's:
>
>   $ make ARCH=i386 defconfig
>   *** Default configuration is based on 'i386_defconfig'
>
>   $ make i386_defconfig
>
> ?





As Nathan explained in this thread,
the prompt of CONFIG_64BIT is hidden
when ARCH=i386.


config 64BIT
        bool "64-bit kernel" if "$(ARCH)" = "x86"
        default "$(ARCH)" != "i386"
        help
          Say yes to build a 64-bit kernel - formerly known as x86_64
          Say no to build a 32-bit kernel - formerly known as i386




[1] "make ARCH=i386 defconfig"

ARCH is i386, so the Kconfig snippet becomes as follows:


  config 64BIT
          bool
          default n




[2] "make i386_defconfig"


ARCH is x86, so the Kconfig snippet becomes as follows:


  config 64BIT
          bool "64-bit kernel"
          default y







"# ... is not set" is output when
the prompt is visible and the user did not enable it.












> Basically the canonical way to generate a defconfig is to provide an ARCH,
> then use the common 'defconfig' target:



I do not know what you mean by "canonical", but
there are only 3 architectures (x86, sparc, sh) that
allow two ways for configuration.

This is somewhat related to a historical reason
(commit 8c4d9b145ba3 mentioned this a little).

After arch/i386/ and arch/x86_64/ were merged into arch/x86,
ARCH=i386 and ARCH=x86_64 became a subset of ARCH=x86.






>
>    make ARCH=i386      defconfig
>    make ARCH=x86       defconfig
>    make ARCH=arm       defconfig
>    make ARCH=arm64     defconfig
>    make ARCH=powerpc   defconfig
>    make ARCH=s390      defconfig
>    make ARCH=sparc     defconfig
>    make ARCH=sparc64   defconfig
>    ...
>
> This is what my build test scripts are using to quick-test cross-arch
> builds. It's a straightforward way to generate defconfigs without knowing
> the details of the myriads of random defconfig targets some architectures
> have, such as:
>
>   kepler:~/tip> ls arch/arm/configs/
>   am200epdkit_defconfig     gemini_defconfig         multi_v5_defconfig    s5pv210_defconfig
>   aspeed_g4_defconfig       h3600_defconfig          multi_v7_defconfig    sama5_defconfig
>   aspeed_g5_defconfig       h5000_defconfig          mv78xx0_defconfig     sama7_defconfig
>   assabet_defconfig         hackkit_defconfig        mvebu_v5_defconfig    shannon_defconfig
>   at91_dt_defconfig         hisi_defconfig           mvebu_v7_defconfig    shmobile_defconfig
>   axm55xx_defconfig         imxrt_defconfig          mxs_defconfig         simpad_defconfig
>   badge4_defconfig          imx_v4_v5_defconfig      neponset_defconfig    socfpga_defconfig
>   bcm2835_defconfig         imx_v6_v7_defconfig      netwinder_defconfig   sp7021_defconfig
>   cerfcube_defconfig        integrator_defconfig     nhk8815_defconfig     spear13xx_defconfig
>   clps711x_defconfig        iop32x_defconfig         omap1_defconfig       spear3xx_defconfig
>   cm_x300_defconfig         ixp4xx_defconfig         omap2plus_defconfig   spear6xx_defconfig
>   cns3420vb_defconfig       jornada720_defconfig     orion5x_defconfig     spitz_defconfig
>   colibri_pxa270_defconfig  keystone_defconfig       oxnas_v6_defconfig    stm32_defconfig
>   colibri_pxa300_defconfig  lart_defconfig           palmz72_defconfig     sunxi_defconfig
>   collie_defconfig          lpc18xx_defconfig        pcm027_defconfig      tct_hammer_defconfig
>   corgi_defconfig           lpc32xx_defconfig        pleb_defconfig        tegra_defconfig
>   davinci_all_defconfig     lpd270_defconfig         pxa168_defconfig      trizeps4_defconfig
>   dove_defconfig            lubbock_defconfig        pxa255-idp_defconfig  u8500_defconfig
>   dram_0x00000000.config    magician_defconfig       pxa3xx_defconfig      versatile_defconfig
>   dram_0xc0000000.config    mainstone_defconfig      pxa910_defconfig      vexpress_defconfig
>   dram_0xd0000000.config    milbeaut_m10v_defconfig  pxa_defconfig         vf610m4_defconfig
>   ep93xx_defconfig          mini2440_defconfig       qcom_defconfig        viper_defconfig
>   eseries_pxa_defconfig     mmp2_defconfig           realview_defconfig    vt8500_v6_v7_defconfig
>   exynos_defconfig          moxart_defconfig         rpc_defconfig         xcep_defconfig
>   ezx_defconfig             mps2_defconfig           s3c2410_defconfig     zeus_defconfig
>   footbridge_defconfig      multi_v4t_defconfig      s3c6400_defconfig
>
> But this doesn't seem to be working reliably on i386:
>
>   kepler:~/tip> make i386_defconfig
>   #
>   # configuration written to .config
>   #
>   kepler:~/tip> /bin/cp .config .config.i386_defconfig.1
>
>   kepler:~/tip> make ARCH=i386 defconfig
>   *** Default configuration is based on 'i386_defconfig'
>   #
>   # configuration written to .config
>   #
>   kepler:~/tip> /bin/cp .config .config.i386_defconfig.2
>
>   kepler:~/tip> diff -up .config.i386_defconfig.1 .config.i386_defconfig.2
>   --- .config.i386_defconfig.1  2022-09-04 11:34:48.253202205 +0200
>   +++ .config.i386_defconfig.2  2022-09-04 11:35:04.268758331 +0200
>   @@ -1,6 +1,6 @@
>    #
>    # Automatically generated file; DO NOT EDIT.
>   -# Linux/x86 6.0.0-rc3 Kernel Configuration
>   +# Linux/i386 6.0.0-rc3 Kernel Configuration
>    #
>    CONFIG_CC_VERSION_TEXT="gcc (Ubuntu 12-20220319-1ubuntu1) 12.0.1 20220319 (experimental) [master r12-7719-g8ca61ad148f]"
>    CONFIG_CC_IS_GCC=y
>   @@ -261,7 +261,6 @@ CONFIG_PROFILING=y
>    CONFIG_TRACEPOINTS=y
>    # end of General setup
>
>   -# CONFIG_64BIT is not set
>    CONFIG_X86_32=y
>    CONFIG_X86=y
>    CONFIG_INSTRUCTION_DECODER=y
>
> Note how in the ARCH=i386 case the build system claims to use i386_defconfig:
>
>   *** Default configuration is based on 'i386_defconfig'
>
> But that doesn't seem to be identical with when we specify i386_defconfig
> directly as a target...
>
> What am I missing?
>
> Thanks,
>
>         Ingo





-- 
Best Regards
Masahiro Yamada

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Ingo Molnar]

* Masahiro Yamada <masahiroy@kernel.org> wrote:

> On Sun, Sep 4, 2022 at 6:48 PM Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > > > Unfortunately, even without the ARCH=i386 'make savedefconfig' doesn't
> > > > seem to be doing the right thing & is dropping the '# CONFIG_64BIT is
> > > > not set' line:
> > >
> > >
> > > Oh, really?
> > >
> > > Without ARCH=i386, it works correctly for me.
> > >
> > >
> > >
> > > masahiro@zoe:~/ref/linux$ make i386_defconfig savedefconfig
> > > #
> > > # No change to .config
> > > #
> > > masahiro@zoe:~/ref/linux$ grep CONFIG_64BIT defconfig
> > > # CONFIG_64BIT is not set
> >
> > Yeah, so why do these two seemingly identical commands produce two
> > different .config's:
> >
> >   $ make ARCH=i386 defconfig
> >   *** Default configuration is based on 'i386_defconfig'
> >
> >   $ make i386_defconfig
> >
> > ?
> 
> 
> 
> As Nathan explained in this thread,
> the prompt of CONFIG_64BIT is hidden
> when ARCH=i386.

Yeah, so my 'why' question was more about why we'd tolerate the UI 
inconsistency in seemingly similar commands generating subtly different 
.config's.

In other words, what should we modify in this Kconfig block, or in other 
parts of the kernel:

> config 64BIT
>         bool "64-bit kernel" if "$(ARCH)" = "x86"
>         default "$(ARCH)" != "i386"
>         help
>           Say yes to build a 64-bit kernel - formerly known as x86_64
>           Say no to build a 32-bit kernel - formerly known as i386

... to not have this Kconfig UI quirk/bug/inconsistency?

I hope we agree that the status quo is not ideal.

Thanks,

	Ingo

Re: [PATCH] x86/config: Make the x86 defconfigs a bit more usable

[Linus Torvalds]

On Fri, Sep 9, 2022 at 6:05 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> If you do not like the inconsistency,
> the support for ARCH=i386 and ARCH=x86_64 must be removed.

Is there any sane alternative for "do cross-architecture test builds"?

I do things like "make ARCH=i386 allmodconfig" myself, and I have to
admit that it's annoying that you have to then remember to do "make
ARCH=ii386" to do the actual build too, because otherwise Kcoinfig
will mess up the architecture, because it didn't make it into the
final simplified .config.

This is probably more noticeable with "defconfig" because then the
"ARCH=xyz" part has *other* effects too (ie it is also what says
*which* defconfig to pick).

So this is a bit of an annoyance, in that it's not trivial to just
"make the config for ARCH xyz" and then "make".

                       Linus

[tip: x86/urgent] x86/defconfig: Enable WERROR

[tip-bot2 for Borislav Petkov]

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     b9080ba4a6ec56447f263082825a4fddb873316b
Gitweb:        https://git.kernel.org/tip/b9080ba4a6ec56447f263082825a4fddb873316b
Author:        Borislav Petkov <bp@suse.de>
AuthorDate:    Wed, 23 Mar 2022 12:21:10 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 24 Mar 2022 09:10:56 +01:00

x86/defconfig: Enable WERROR

To quote Linus:

  "EVERYBODY should have CONFIG_WERROR=y on at least x86-64 and other
   serious architectures, unless you have some completely random
   experimental (and broken) compiler.

New compiler warnings are not acceptable."

So this should make at least the most obvious and common ones not go
unnoticed.

Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/YjsCpoRK7W4l6tSh@zn.tnic
---
 arch/x86/configs/i386_defconfig   | 1 +
 arch/x86/configs/x86_64_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig
index 71124cf..4b20852 100644
--- a/arch/x86/configs/i386_defconfig
+++ b/arch/x86/configs/i386_defconfig
@@ -262,3 +262,4 @@ CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
 CONFIG_EARLY_PRINTK_DBGP=y
 CONFIG_DEBUG_BOOT_PARAMS=y
 CONFIG_KALLSYMS_ALL=y
+CONFIG_WERROR=y
diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig
index 92b1169..38c52e4 100644
--- a/arch/x86/configs/x86_64_defconfig
+++ b/arch/x86/configs/x86_64_defconfig
@@ -258,3 +258,4 @@ CONFIG_PROVIDE_OHCI1394_DMA_INIT=y
 CONFIG_EARLY_PRINTK_DBGP=y
 CONFIG_DEBUG_BOOT_PARAMS=y
 CONFIG_KALLSYMS_ALL=y
+CONFIG_WERROR=y

Re: [GIT PULL] locking changes for v5.18

[Andy Shevchenko]

On Tue, Mar 22, 2022 at 03:58:47PM -0700, Linus Torvalds wrote:
> On Tue, Mar 22, 2022 at 3:20 PM Borislav Petkov <bp@alien8.de> wrote:
> >
> > Ah, you say build error because you have CONFIG_WERROR=y.
> 
> EVERYBODY should have CONFIG_WERROR=y on at least x86-64 and other
> serious architectures, unless you have some completely random
> experimental (and broken) compiler.
> 
> New compiler warnings are not acceptable.

What about old one? I have already complained in the early discussion that
`make W=1 ...` is broken by this change. Enabling it without fixing
_existing_ warnings on W=1 is not suitable for somebody. Now, I have to
modify my configs to disable WERROR because of inability to built at all.

(Yes, I understand that I may drop W=1, but that's not the point. since I
 want to have clean builds of a new code on level 1 of warnings)

-- 
With Best Regards,
Andy Shevchenko

Re: [GIT PULL] locking changes for v5.18

[Peter Zijlstra]

On Fri, Mar 25, 2022 at 01:41:25PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 22, 2022 at 03:58:47PM -0700, Linus Torvalds wrote:
> > On Tue, Mar 22, 2022 at 3:20 PM Borislav Petkov <bp@alien8.de> wrote:
> > >
> > > Ah, you say build error because you have CONFIG_WERROR=y.
> > 
> > EVERYBODY should have CONFIG_WERROR=y on at least x86-64 and other
> > serious architectures, unless you have some completely random
> > experimental (and broken) compiler.
> > 
> > New compiler warnings are not acceptable.
> 
> What about old one? I have already complained in the early discussion that
> `make W=1 ...` is broken by this change. Enabling it without fixing
> _existing_ warnings on W=1 is not suitable for somebody. Now, I have to
> modify my configs to disable WERROR because of inability to built at all.
> 
> (Yes, I understand that I may drop W=1, but that's not the point. since I
>  want to have clean builds of a new code on level 1 of warnings)

It would be fairly easy to make scripts/Makefile.extrawarn strip out
-Werror when W= is used.

Re: [GIT PULL] locking changes for v5.18

[Andy Shevchenko]

On Fri, Mar 25, 2022 at 01:23:36PM +0100, Peter Zijlstra wrote:
> On Fri, Mar 25, 2022 at 01:41:25PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 22, 2022 at 03:58:47PM -0700, Linus Torvalds wrote:
> > > On Tue, Mar 22, 2022 at 3:20 PM Borislav Petkov <bp@alien8.de> wrote:
> > > >
> > > > Ah, you say build error because you have CONFIG_WERROR=y.
> > > 
> > > EVERYBODY should have CONFIG_WERROR=y on at least x86-64 and other
> > > serious architectures, unless you have some completely random
> > > experimental (and broken) compiler.
> > > 
> > > New compiler warnings are not acceptable.
> > 
> > What about old one? I have already complained in the early discussion that
> > `make W=1 ...` is broken by this change. Enabling it without fixing
> > _existing_ warnings on W=1 is not suitable for somebody. Now, I have to
> > modify my configs to disable WERROR because of inability to built at all.
> > 
> > (Yes, I understand that I may drop W=1, but that's not the point. since I
> >  want to have clean builds of a new code on level 1 of warnings)
> 
> It would be fairly easy to make scripts/Makefile.extrawarn strip out
> -Werror when W= is used.

Hmm... I can't achieve this, because it complains about recursive variable.
What helped me is to supply in such case -Wno-error which seems overrode
the previous setting.

I'll send a patch to discuss further if needed.

-- 
With Best Regards,
Andy Shevchenko

Re: [GIT PULL] locking changes for v5.18

[Linus Torvalds]

On Fri, Mar 25, 2022 at 4:42 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> What about old one? I have already complained in the early discussion that
> `make W=1 ...` is broken by this change.

So that is REALLY D*MN EASY TO FIX.

If you use W=1, and don't want WERROR, then don't *do* that then.

End of story.

But that's on _you_. Not on the build system. If you use W=1 and
WERROR together, you get exactly what you asked for. It might even be
what you wanted, if you want to go through the warnings/errors as you
encounter them, instead of building everything.

And that's why I refuse to take the completely broken "strip out one
or the other automatically" change.

It's a perfectly valid combination to enable both.

But more importantly, -Werror is more important than W=1. So if
anything should be disabled, it's W=1.

Side note: that would be trivial to just have in the Kconfig files if
W=1 was just a config option.

Do something like

     config EXTRA_ERRORS
         int "Add extra compiler errors" if EXPERT
         depends on !WERROR
         range 0-2
         default 0

but note again: WERROR should be the thing that controls this and
should be on by default, not the other way around.

If you want EXTRA_ERRORS, you should not only be CONFIG_EXPERT, you
should also have to manually disable WERROR that *normal* people
should have on by default.

              Linus

Re: [GIT PULL] locking changes for v5.18

[Andy Shevchenko]

On Fri, Mar 25, 2022 at 10:29:21AM -0700, Linus Torvalds wrote:
> On Fri, Mar 25, 2022 at 4:42 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > What about old one? I have already complained in the early discussion that
> > `make W=1 ...` is broken by this change.
> 
> So that is REALLY D*MN EASY TO FIX.
> 
> If you use W=1, and don't want WERROR, then don't *do* that then.
> 
> End of story.
> 
> But that's on _you_. Not on the build system. If you use W=1 and
> WERROR together, you get exactly what you asked for. It might even be
> what you wanted, if you want to go through the warnings/errors as you
> encounter them, instead of building everything.
> 
> And that's why I refuse to take the completely broken "strip out one
> or the other automatically" change.
> 
> It's a perfectly valid combination to enable both.
> 
> But more importantly, -Werror is more important than W=1. So if
> anything should be disabled, it's W=1.
> 
> Side note: that would be trivial to just have in the Kconfig files if
> W=1 was just a config option.
> 
> Do something like
> 
>      config EXTRA_ERRORS
>          int "Add extra compiler errors" if EXPERT
>          depends on !WERROR
>          range 0-2
>          default 0
> 
> but note again: WERROR should be the thing that controls this and
> should be on by default, not the other way around.
> 
> If you want EXTRA_ERRORS, you should not only be CONFIG_EXPERT, you
> should also have to manually disable WERROR that *normal* people
> should have on by default.

I have got your point, thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [GIT PULL] locking changes for v5.18

[Peter Zijlstra]

On Tue, Mar 22, 2022 at 03:05:39PM -0700, Linus Torvalds wrote:

> I thought -tip had started checking with clang, but apparently not.

Yeah, sorry. I do indeed not regularly build with clang and somewhat
rely on the robots to yell at me.

Every time I try clang, it just takes so long.. build times are
significantly longer. On top of that our build system hasn't, until
recently [1], much liked how Debian packages clang.

[1] https://lkml.kernel.org/r/20220304170813.1689186-1-nathan@kernel.org

Re: [GIT PULL] locking changes for v5.18

[Linus Torvalds]

On Tue, Mar 22, 2022 at 3:05 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I see that the -mm tree has a fix for this,[..]

Ok, usually I strive to let the patch-bomb from Andrew sit in a branch
of its own for a while (in case somebody replies to one of Andrew's
emails I can then fix things up).

But I decided to just apply and merge the series immediately instead,
partly to just have this issue sorted out.

Let's hope there's nothing dodgy in there..

               Linus

Re: [GIT PULL] locking changes for v5.18

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Mar 22, 2022 at 3:05 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > I see that the -mm tree has a fix for this,[..]
> 
> Ok, usually I strive to let the patch-bomb from Andrew sit in a branch
> of its own for a while (in case somebody replies to one of Andrew's
> emails I can then fix things up).
> 
> But I decided to just apply and merge the series immediately instead,
> partly to just have this issue sorted out.
> 
> Let's hope there's nothing dodgy in there..

Thanks!

> > I thought -tip had started checking with clang, but apparently not.

Only intermittently on my side - it only recently started working reliably 
& it doubles the not inconsiderable test time :-/

> > I see that the -mm tree has a fix for this, but I'm rather unhappy that 
> > the -tip tree build checking has deteriorated so much, and clang builds 
> > will now have a pointless build error that will cause issues for 
> > bisect.

Will try to add better clang testing, seems to have better warnings in a 
couple of areas.

Thanks,

	Ingo

Re: [GIT PULL] locking changes for v5.18

[Borislav Petkov]

On Thu, Mar 24, 2022 at 09:40:24AM +0100, Ingo Molnar wrote:
> Only intermittently on my side - it only recently started working
> reliably & it doubles the not inconsiderable test time :-/

True dat - last time I measured, clang builds take roughly double the
time gcc builds with the same config do.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [GIT PULL] locking changes for v5.18

[Nick Desaulniers]

On Thu, Mar 24, 2022 at 11:19:58AM +0100, Borislav Petkov wrote:
> On Thu, Mar 24, 2022 at 09:40:24AM +0100, Ingo Molnar wrote:
> > Only intermittently on my side - it only recently started working
> > reliably & it doubles the not inconsiderable test time :-/
> 
> True dat - last time I measured, clang builds take roughly double the
> time gcc builds with the same config do.

See also:
https://lore.kernel.org/lkml/CAHk-=whs8QZf3YnifdLv57+FhBi5_WeNTG1B-suOES=RcUSmQg@mail.gmail.com/

We do have to pay a penalty in that if these binaries can be built as
shared objects, the MUST be. (Actually, IDK if Debian has the same
policy as Fedora).

If you see libLLVM-*.so (and libclang-cpp.so) in your list of
`ldd $(which clang)`, or see multiple symbols from the dynamic linker
after running:

$ perf record -e cycles:pp --call-graph lbr make LLVM=1
$ perf report --no-children --sort=dso,cycles

Then dynamic linking is the main source of performance difference
between GCC and Clang on those distributions. So we're kind of fighting
with one hand behind our backs.

For Linux distros that have server costs, my guess is these policies
allow them to minimize their costs of serving prebuilt binaries, since
dynamic linking does allow for smaller binaries.

For short lived processes like most of the toolchain (and parts that
generally don't overlap, such as the serialization between compilation
THEN linkages), dynamic linking provides a ridiculous startup cost. CoW
gets us most of the benefits of dynamic linking in terms of shared
memory between processes. (I don't think ASLR hurts CoW in that way?)

On the flip side, I have been banging pots and pans around internally
trying to raise awareness of the issue (I'm but a peasant though, Mr.
bottom rung of the ladder); we do have budget finally allocated to focus
on compiler performance. Getting the checks written involves more red
tape and more effort than I expected.

I might start ignoring compiler bugs for a while, go off into the
wilderness, and not come back until I have statically linked images of
clang available to host on kernel.org though. There's a lot we could be
doing to hot rod the compiler to optimize for developer's (and CI's)
time.

Re: [GIT PULL] locking changes for v5.18

[pr-tracker-bot]

The pull request you sent on Mon, 21 Mar 2022 12:11:44 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-core-2022-03-21

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ebd326ce724d5b2e5274724e6d6a46a046e28203

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html