public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] locking changes for v6.13
@ 2024-11-18  9:03 Ingo Molnar
  2024-11-19 20:56 ` Linus Torvalds
  2024-11-19 23:33 ` [GIT PULL] locking changes for v6.13 pr-tracker-bot
  0 siblings, 2 replies; 5+ messages in thread
From: Ingo Molnar @ 2024-11-18  9:03 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Peter Zijlstra, Thomas Gleixner, Will Deacon,
	Waiman Long, Boqun Feng, Borislav Petkov

Linus,

Please pull the latest locking/core Git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-core-2024-11-18

   # HEAD: 3b49a347d751553b1d1be69c8619ae2e85fdc28d locking/Documentation: Fix grammar in percpu-rw-semaphore.rst

Locking changes for v6.13 are:

 - lockdep:
    - Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING (Sebastian Andrzej Siewior)
    - Add lockdep_cleanup_dead_cpu() (David Woodhouse)

 - futexes:
    - Use atomic64_inc_return() in get_inode_sequence_number() (Uros Bizjak)
    - Use atomic64_try_cmpxchg_relaxed() in get_inode_sequence_number() (Uros Bizjak)

 - RT locking:
    - Add sparse annotation PREEMPT_RT's locking (Sebastian Andrzej Siewior)

 - spinlocks:
    - Use atomic_try_cmpxchg_release() in osq_unlock() (Uros Bizjak)

 - atomics:
    - x86: Use ALT_OUTPUT_SP() for __alternative_atomic64() (Uros Bizjak)
    - x86: Use ALT_OUTPUT_SP() for __arch_{,try_}cmpxchg64_emu() (Uros Bizjak)

 - KCSAN, seqlocks:
    - Support seqcount_latch_t (Marco Elver)

 - <linux/cleanup.h>:
    - Add if_not_cond_guard() conditional guard helper (David Lechner)
    - Adjust scoped_guard() macros to avoid potential warning (Przemek Kitszel)
    - Remove address space of returned pointer (Uros Bizjak)

 - WW mutexes:
    - locking/ww_mutex: Adjust to lockdep nest_lock requirements (Thomas Hellström)

 - Rust integration:
    - Fix raw_spin_lock initialization on PREEMPT_RT (Eder Zulian)

 - miscellaneous cleanups & fixes:
    - lockdep: Fix wait-type check related warnings (Ahmed Ehab)
    - lockdep: Use info level for initial info messages (Jiri Slaby)
    - spinlocks: Make __raw_* lock ops static (Geert Uytterhoeven)
    - pvqspinlock: Convert fields of 'enum vcpu_state' to uppercase (Qiuxu Zhuo)
    - iio: magnetometer: Fix if () scoped_guard() formatting (Stephen Rothwell)
    - rtmutex: Fix misleading comment (Peter Zijlstra)
    - percpu-rw-semaphores: Fix grammar in percpu-rw-semaphore.rst (Xiu Jianfeng)

 Thanks,

	Ingo

------------------>
Ahmed Ehab (2):
      locking/lockdep: Avoid creating new name string literals in lockdep_set_subclass()
      locking/lockdep: Add a test for lockdep_set_subclass()

David Lechner (1):
      cleanup: Add conditional guard helper

David Woodhouse (1):
      lockdep: Add lockdep_cleanup_dead_cpu()

Eder Zulian (1):
      rust: helpers: Avoid raw_spin_lock initialization for PREEMPT_RT

Geert Uytterhoeven (1):
      locking/spinlocks: Make __raw_* lock ops static

Jiri Slaby (SUSE) (1):
      lockdep: Use info level for lockdep initial info messages

Marco Elver (5):
      time/sched_clock: Swap update_clock_read_data() latch writes
      time/sched_clock: Broaden sched_clock()'s instrumentation coverage
      kcsan, seqlock: Support seqcount_latch_t
      seqlock, treewide: Switch to non-raw seqcount_latch interface
      kcsan, seqlock: Fix incorrect assumption in read_seqbegin()

Peter Zijlstra (1):
      locking/rtmutex: Fix misleading comment

Przemek Kitszel (1):
      cleanup: Adjust scoped_guard() macros to avoid potential warning

Qiuxu Zhuo (1):
      locking/pvqspinlock: Convert fields of 'enum vcpu_state' to uppercase

Sebastian Andrzej Siewior (5):
      lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.
      locking/rt: Add sparse annotation PREEMPT_RT's sleeping locks.
      locking/rt: Remove one __cond_lock() in RT's spin_trylock_irqsave()
      locking/rt: Add sparse annotation for RCU.
      locking/rt: Annotate unlock followed by lock for sparse.

Stephen Rothwell (1):
      iio: magnetometer: fix if () scoped_guard() formatting

Thomas Hellström (1):
      locking/ww_mutex: Adjust to lockdep nest_lock requirements

Uros Bizjak (6):
      futex: Use atomic64_inc_return() in get_inode_sequence_number()
      futex: Use atomic64_try_cmpxchg_relaxed() in get_inode_sequence_number()
      cleanup: Remove address space of returned pointer
      locking/osq_lock: Use atomic_try_cmpxchg_release() in osq_unlock()
      locking/atomic/x86: Use ALT_OUTPUT_SP() for __alternative_atomic64()
      locking/atomic/x86: Use ALT_OUTPUT_SP() for __arch_{,try_}cmpxchg64_emu()

Xiu Jianfeng (1):
      locking/Documentation: Fix grammar in percpu-rw-semaphore.rst


 Documentation/locking/percpu-rw-semaphore.rst |  4 +-
 Documentation/locking/seqlock.rst             |  2 +-
 arch/x86/include/asm/atomic64_32.h            |  3 +-
 arch/x86/include/asm/cmpxchg_32.h             |  6 +-
 arch/x86/kernel/tsc.c                         |  5 +-
 drivers/iio/magnetometer/af8133j.c            |  3 +-
 include/linux/cleanup.h                       | 69 ++++++++++++++++---
 include/linux/irqflags.h                      |  6 ++
 include/linux/lockdep.h                       |  2 +-
 include/linux/rbtree_latch.h                  | 20 +++---
 include/linux/rwlock_rt.h                     | 10 +--
 include/linux/seqlock.h                       | 98 ++++++++++++++++++++-------
 include/linux/spinlock_rt.h                   | 28 ++++----
 include/linux/ww_mutex.h                      | 14 ++++
 kernel/cpu.c                                  |  1 +
 kernel/futex/core.c                           |  6 +-
 kernel/locking/lockdep.c                      | 46 ++++++++++---
 kernel/locking/osq_lock.c                     |  3 +-
 kernel/locking/qspinlock_paravirt.h           | 36 +++++-----
 kernel/locking/rtmutex.c                      |  2 +
 kernel/locking/rtmutex_api.c                  |  8 +--
 kernel/locking/spinlock.c                     |  8 +--
 kernel/locking/spinlock_rt.c                  | 14 ++--
 kernel/locking/test-ww_mutex.c                |  8 ++-
 kernel/printk/printk.c                        |  9 +--
 kernel/time/sched_clock.c                     | 34 +++++++---
 kernel/time/timekeeping.c                     | 12 ++--
 lib/Kconfig.debug                             | 12 +---
 lib/locking-selftest.c                        | 39 +++++++++++
 rust/helpers/spinlock.c                       |  8 ++-
 30 files changed, 355 insertions(+), 161 deletions(-)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [GIT PULL] locking changes for v6.13
  2024-11-18  9:03 [GIT PULL] locking changes for v6.13 Ingo Molnar
@ 2024-11-19 20:56 ` Linus Torvalds
  2024-11-20  0:02   ` Ingo Molnar
  2024-11-20 11:36   ` [PATCH] headers/cleanup.h: Fix if_not_guard() fragility Ingo Molnar
  2024-11-19 23:33 ` [GIT PULL] locking changes for v6.13 pr-tracker-bot
  1 sibling, 2 replies; 5+ messages in thread
From: Linus Torvalds @ 2024-11-19 20:56 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Peter Zijlstra, Thomas Gleixner, Will Deacon,
	Waiman Long, Boqun Feng, Borislav Petkov

On Mon, 18 Nov 2024 at 01:03, Ingo Molnar <mingo@kernel.org> wrote:
>
>  - <linux/cleanup.h>:
>     - Add if_not_cond_guard() conditional guard helper (David Lechner)

I've pulled this, but I'm unhappy.

This macro generates actively wrong code if it happens to be inside an
if-statement or a loop without a block.

IOW, code like this:

    for (iterate-over-something)
        if_not_guard(a)
            return -BUSY;

looks like will build fine, but will generate completely incorrect code.

Honestly, just switching the order of the BUILD_BUG_ON() and the
CLASS() declaration looks like it would have fixed this (because then
the '_id' won't be in scope of the subsequent if-statement any more),
but I'm unhappy with how apparently nobody even bothered to think
about such a fundamental issue with macros.

Macros that expand to statements absolutely *ALWAYS* need to deal with
"what if we're in a single-statement situation?"

              Linus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [GIT PULL] locking changes for v6.13
  2024-11-18  9:03 [GIT PULL] locking changes for v6.13 Ingo Molnar
  2024-11-19 20:56 ` Linus Torvalds
@ 2024-11-19 23:33 ` pr-tracker-bot
  1 sibling, 0 replies; 5+ messages in thread
From: pr-tracker-bot @ 2024-11-19 23:33 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, linux-kernel, Peter Zijlstra, Thomas Gleixner,
	Will Deacon, Waiman Long, Boqun Feng, Borislav Petkov

The pull request you sent on Mon, 18 Nov 2024 10:03:40 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-core-2024-11-18

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

Thank you!

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [GIT PULL] locking changes for v6.13
  2024-11-19 20:56 ` Linus Torvalds
@ 2024-11-20  0:02   ` Ingo Molnar
  2024-11-20 11:36   ` [PATCH] headers/cleanup.h: Fix if_not_guard() fragility Ingo Molnar
  1 sibling, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2024-11-20  0:02 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Peter Zijlstra, Thomas Gleixner, Will Deacon,
	Waiman Long, Boqun Feng, Borislav Petkov


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

> On Mon, 18 Nov 2024 at 01:03, Ingo Molnar <mingo@kernel.org> wrote:
> >
> >  - <linux/cleanup.h>:
> >     - Add if_not_cond_guard() conditional guard helper (David Lechner)
> 
> I've pulled this, but I'm unhappy.
> 
> This macro generates actively wrong code if it happens to be inside an
> if-statement or a loop without a block.
> 
> IOW, code like this:
> 
>     for (iterate-over-something)
>         if_not_guard(a)
>             return -BUSY;
> 
> looks like will build fine, but will generate completely incorrect code.
> 
> Honestly, just switching the order of the BUILD_BUG_ON() and the
> CLASS() declaration looks like it would have fixed this (because then
> the '_id' won't be in scope of the subsequent if-statement any more),
> but I'm unhappy with how apparently nobody even bothered to think
> about such a fundamental issue with macros.
> 
> Macros that expand to statements absolutely *ALWAYS* need to deal with
> "what if we're in a single-statement situation?"

Indeed - sorry about that, will sort this out tomorrow.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] headers/cleanup.h: Fix if_not_guard() fragility
  2024-11-19 20:56 ` Linus Torvalds
  2024-11-20  0:02   ` Ingo Molnar
@ 2024-11-20 11:36   ` Ingo Molnar
  1 sibling, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2024-11-20 11:36 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, Peter Zijlstra, Thomas Gleixner, Will Deacon,
	Waiman Long, Boqun Feng, Borislav Petkov, David Lechner


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

> On Mon, 18 Nov 2024 at 01:03, Ingo Molnar <mingo@kernel.org> wrote:
> >
> >  - <linux/cleanup.h>:
> >     - Add if_not_cond_guard() conditional guard helper (David Lechner)
> 
> I've pulled this, but I'm unhappy.
> 
> This macro generates actively wrong code if it happens to be inside an
> if-statement or a loop without a block.
> 
> IOW, code like this:
> 
>     for (iterate-over-something)
>         if_not_guard(a)
>             return -BUSY;
> 
> looks like will build fine, but will generate completely incorrect code.
> 
> Honestly, just switching the order of the BUILD_BUG_ON() and the
> CLASS() declaration looks like it would have fixed this (because then
> the '_id' won't be in scope of the subsequent if-statement any more),
> but I'm unhappy with how apparently nobody even bothered to think
> about such a fundamental issue with macros.
> 
> Macros that expand to statements absolutely *ALWAYS* need to deal with
> "what if we're in a single-statement situation?"

How about the fix below?

Thanks,

	Ingo

=======================>
From: Ingo Molnar <mingo@kernel.org>
Date: Wed, 20 Nov 2024 11:56:31 +0100
Subject: [PATCH] headers/cleanup.h: Fix if_not_guard() fragility

Linus noticed that the new if_not_guard() definition is fragile:

   "This macro generates actively wrong code if it happens to be inside an
    if-statement or a loop without a block.

    IOW, code like this:

      for (iterate-over-something)
          if_not_guard(a)
              return -BUSY;

    looks like will build fine, but will generate completely incorrect code."

The reason is that the __if_not_guard() macro is multi-statement, so 
while most kernel developers expect macros to be simple or at least 
compound statements - but for __if_not_guard() it is not so:

 #define __if_not_guard(_name, _id, args...)            \
        BUILD_BUG_ON(!__is_cond_ptr(_name));            \
        CLASS(_name, _id)(args);                        \
        if (!__guard_ptr(_name)(&_id))

To add insult to injury, the placement of the BUILD_BUG_ON() line makes 
the macro appear to compile fine, but it will generate incorrect code 
as Linus reported, for example if used within iteration or conditional 
statements that will use the first statement of a macro as a loop body 
or conditional statement body.

While it doesn't appear to be possible to turn this macro into a robust 
single or compound statement that could be used in single statements, 
due to the necessity to define an auto scope variable with an open 
scope and the necessity of it having to expand to a partial 'if' 
statement with no body - we can at least make sure the macro won't 
build if used in a single-statement construct: such as by making the 
CLASS() line the first statement in the macro, followed by the other 
statements, which would break the build, as the single statement would 
close the scope.

Do this.

To test this, I added an artificial if_not_guard() usecase within a 
single statement:

Before:

	$ make kernel/ptrace.o
	CC      kernel/ptrace.o
	$

After:

	CC      kernel/ptrace.o
	In file included from ./include/linux/irqflags.h:17,
		       from ./arch/x86/include/asm/special_insns.h:10,
		       from ./arch/x86/include/asm/processor.h:25,
		       from ./include/linux/sched.h:13,
		       from kernel/ptrace.c:13:
	kernel/ptrace.c: In function ‘ptrace_attach’:
	./include/linux/cleanup.h:258:9: error: expected expression before ‘class_mutex_intr_t’

I'd also like to note that the original submission by David Lechner did 
not contain the BUILD_BUG_ON() line, so it was safer than what we ended 
up committing. Mea culpa.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: David Lechner <dlechner@baylibre.com>
Fixes: 36c2cf88808d cleanup: Add conditional guard helper
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/cleanup.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index 966fcc5ff8ef..263f14085617 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -351,8 +351,8 @@ _label:									\
 	__scoped_cond_guard(_name, _fail, __UNIQUE_ID(label), args)
 
 #define __if_not_guard(_name, _id, args...)		\
-	BUILD_BUG_ON(!__is_cond_ptr(_name));		\
 	CLASS(_name, _id)(args);			\
+	BUILD_BUG_ON(!__is_cond_ptr(_name));		\
 	if (!__guard_ptr(_name)(&_id))
 
 #define if_not_guard(_name, args...) \

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-11-20 11:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-18  9:03 [GIT PULL] locking changes for v6.13 Ingo Molnar
2024-11-19 20:56 ` Linus Torvalds
2024-11-20  0:02   ` Ingo Molnar
2024-11-20 11:36   ` [PATCH] headers/cleanup.h: Fix if_not_guard() fragility Ingo Molnar
2024-11-19 23:33 ` [GIT PULL] locking changes for v6.13 pr-tracker-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox