[GIT PULL] locking changes for v6.2

[GIT PULL] locking changes for v6.2

[Ingo Molnar]

Linus,

Please pull the latest locking tree from:

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

   # HEAD: 90d758896787048fa3d4209309d4800f3920e66f futex: Resend potentially swallowed owner death notification

Two changes in this cycle:

 - a micro-optimization in static_key_slow_inc_cpuslocked()
 - fix futex death-notification wakeup bug

 Thanks,

	Ingo

------------------>
Alexey Izbyshev (1):
      futex: Resend potentially swallowed owner death notification

Uros Bizjak (1):
      jump_label: Use atomic_try_cmpxchg() in static_key_slow_inc_cpuslocked()


 kernel/futex/core.c | 26 +++++++++++++++++---------
 kernel/jump_label.c |  8 ++------
 2 files changed, 19 insertions(+), 15 deletions(-)

Re: [GIT PULL] locking changes for v6.2

[pr-tracker-bot]

The pull request you sent on Mon, 12 Dec 2022 21:16:13 +0100:

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

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

Thank you!

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

[GIT PULL] locking changes for v6.3

[Ingo Molnar]

Linus,

Please pull the latest locking tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-core-2023-02-20

   # HEAD: 3b4863fa5b7dd50dab1b10abbed938efd203752f vduse: Remove include of rwlock.h

Updates for this cycle were:

 - rwsem micro-optimizations
 - spinlock micro-optimizations
 - cleanups, simplifications

 Thanks,

	Ingo

------------------>
Guo Ren (1):
      locking/qspinlock: Micro-optimize pending state waiting for unlock

Sebastian Andrzej Siewior (2):
      locking/lockdep: Remove lockdep_init_map_crosslock.
      vduse: Remove include of rwlock.h

Uros Bizjak (2):
      x86/PAT: Use try_cmpxchg() in set_page_memtype()
      x86/ACPI/boot: Use try_cmpxchg() in __acpi_{acquire,release}_global_lock()

Waiman Long (3):
      locking/rwsem: Prevent non-first waiter from spinning in down_write() slowpath
      locking/rwsem: Disable preemption in all down_read*() and up_read() code paths
      locking/rwsem: Disable preemption in all down_write*() and up_write() code paths


 arch/x86/kernel/acpi/boot.c          | 16 +++----
 arch/x86/mm/pat/memtype.c            |  4 +-
 drivers/vdpa/vdpa_user/iova_domain.h |  1 -
 include/linux/lockdep.h              |  1 -
 kernel/locking/qspinlock.c           |  4 +-
 kernel/locking/rwsem.c               | 87 +++++++++++++++++++++---------------
 6 files changed, 64 insertions(+), 49 deletions(-)

Re: [GIT PULL] locking changes for v6.3

[pr-tracker-bot]

The pull request you sent on Mon, 20 Feb 2023 13:25:50 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking-core-2023-02-20

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

Thank you!

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

[GIT PULL] locking changes for v6.4

[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-2023-04-27

   # HEAD: 93eff603d6a2bc1895eeb7063dbd0661bb760b74 locking/atomic: Correct (cmp)xhcg instrumentation

Locking changes in v6.4:

 - Introduce local{,64}_try_cmpxchg() - a slightly more optimal
   primitive, which will be used in perf events ring-buffer code.

 - Add more atomic_add_negative() variants, used by rcuref:

 - Add rcuref - A scalable reference count implementation for RCU managed objects

 - Add non-atomic __xchg() variant, use it in a couple of places

 - Misc cleanups and fixes.


NOTE: the internal merge commit of locking/rcuref [1afa833d607b6] merging 
      two commits from a WIP branch has no explanation. I noticed this too
      late, and we can re-do this tree if it's an issue.

 Thanks,

	Ingo

------------------>
Andrzej Hajda (5):
      arch: rename all internal names __xchg to __arch_xchg
      linux/include: add non-atomic version of xchg
      arch/*/uprobes: simplify arch_uretprobe_hijack_return_addr
      llist: simplify __llist_del_all
      drm/i915/gt: use __xchg instead of internal helper

Mark Rutland (1):
      locking/atomic: Correct (cmp)xhcg instrumentation

Sebastian Andrzej Siewior (1):
      locking/rwbase: Mitigate indefinite writer starvation.

Thomas Gleixner (2):
      atomics: Provide atomic_add_negative() variants
      atomics: Provide rcuref - scalable reference counting

Uros Bizjak (4):
      locking/atomic: Add generic try_cmpxchg{,64}_local support
      locking/generic: Wire up local{,64}_try_cmpxchg
      locking/arch: Wire up local_try_cmpxchg
      locking/x86: Define arch_try_cmpxchg_local


 arch/alpha/include/asm/cmpxchg.h                   |  10 +-
 arch/alpha/include/asm/local.h                     |  12 +-
 arch/arc/include/asm/cmpxchg.h                     |   4 +-
 arch/arm/include/asm/cmpxchg.h                     |   7 +-
 arch/arm/probes/uprobes/core.c                     |   8 +-
 arch/arm64/include/asm/cmpxchg.h                   |   7 +-
 arch/arm64/kernel/probes/uprobes.c                 |   9 +-
 arch/csky/kernel/probes/uprobes.c                  |   9 +-
 arch/hexagon/include/asm/cmpxchg.h                 |  10 +-
 arch/ia64/include/asm/cmpxchg.h                    |   2 +-
 arch/ia64/include/uapi/asm/cmpxchg.h               |   4 +-
 arch/loongarch/include/asm/cmpxchg.h               |   4 +-
 arch/loongarch/include/asm/local.h                 |  13 +-
 arch/m68k/include/asm/cmpxchg.h                    |   6 +-
 arch/mips/include/asm/cmpxchg.h                    |   4 +-
 arch/mips/include/asm/local.h                      |  13 +-
 arch/mips/kernel/uprobes.c                         |  10 +-
 arch/openrisc/include/asm/cmpxchg.h                |  10 +-
 arch/parisc/include/asm/cmpxchg.h                  |   4 +-
 arch/powerpc/include/asm/cmpxchg.h                 |   4 +-
 arch/powerpc/include/asm/local.h                   |  11 +
 arch/powerpc/kernel/uprobes.c                      |  10 +-
 arch/riscv/include/asm/atomic.h                    |   2 +-
 arch/riscv/include/asm/cmpxchg.h                   |   4 +-
 arch/riscv/kernel/probes/uprobes.c                 |   9 +-
 arch/s390/include/asm/cmpxchg.h                    |   8 +-
 arch/s390/kernel/uprobes.c                         |   7 +-
 arch/sh/include/asm/cmpxchg.h                      |   4 +-
 arch/sparc/include/asm/cmpxchg_32.h                |   4 +-
 arch/sparc/include/asm/cmpxchg_64.h                |   6 +-
 arch/sparc/kernel/uprobes.c                        |   7 +-
 arch/x86/include/asm/cmpxchg.h                     |   6 +
 arch/x86/include/asm/local.h                       |  13 +-
 arch/xtensa/include/asm/cmpxchg.h                  |   4 +-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c          |   2 +-
 drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c   |   4 +-
 .../gpu/drm/i915/gt/intel_execlists_submission.c   |   4 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c               |   4 +-
 drivers/gpu/drm/i915/gt/intel_gsc.c                |   2 +-
 drivers/gpu/drm/i915/gt/intel_gt.c                 |   4 +-
 drivers/gpu/drm/i915/gt/intel_gt_pm.c              |   2 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c                |   6 +-
 drivers/gpu/drm/i915/gt/intel_migrate.c            |   2 +-
 drivers/gpu/drm/i915/gt/intel_rc6.c                |   2 +-
 drivers/gpu/drm/i915/gt/intel_rps.c                |   2 +-
 drivers/gpu/drm/i915/gt/selftest_context.c         |   2 +-
 drivers/gpu/drm/i915/gt/selftest_ring_submission.c |   2 +-
 drivers/gpu/drm/i915/gt/selftest_timeline.c        |   2 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc.c              |   2 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c           |   2 +-
 drivers/gpu/drm/i915/i915_utils.h                  |   1 +
 include/asm-generic/local.h                        |   1 +
 include/asm-generic/local64.h                      |  12 +-
 include/linux/atomic/atomic-arch-fallback.h        | 230 ++++++++++++++++-
 include/linux/atomic/atomic-instrumented.h         | 152 ++++++++---
 include/linux/atomic/atomic-long.h                 |  38 ++-
 include/linux/llist.h                              |   6 +-
 include/linux/non-atomic/xchg.h                    |  19 ++
 include/linux/rcuref.h                             | 155 ++++++++++++
 include/linux/types.h                              |   6 +
 kernel/locking/rwbase_rt.c                         |   9 -
 lib/Makefile                                       |   2 +-
 lib/rcuref.c                                       | 281 +++++++++++++++++++++
 scripts/atomic/atomics.tbl                         |   2 +-
 scripts/atomic/fallbacks/add_negative              |  11 +-
 scripts/atomic/gen-atomic-fallback.sh              |   4 +
 scripts/atomic/gen-atomic-instrumented.sh          |   8 +-
 67 files changed, 1019 insertions(+), 207 deletions(-)
 create mode 100644 include/linux/non-atomic/xchg.h
 create mode 100644 include/linux/rcuref.h
 create mode 100644 lib/rcuref.c

Re: [GIT PULL] locking changes for v6.4

[Linus Torvalds]

On Thu, Apr 27, 2023 at 12:58 PM Ingo Molnar <mingo@kernel.org> wrote:
>
>  - Add non-atomic __xchg() variant, use it in a couple of places

Guys, this is insane, and completely unacceptable.

I pulled this, but I'm going to unpull it, because the code is
actively wrong and ugly.

It not only randomly decides to re-use a name that has existing users
that now need to be fixed up.

It then *also* decides to start "preferring" this absolutely
disgusting new name over a much more legible one in the i915 driver,
which had this same functionality except it used a prettier name:

   fetch_and_zero()

But what then takes the cake for me is that this horribly ugly feature
then didn't even get that right, and only randomly converted *some* of
the users, with most of them remaining:

  git grep fetch_and_zero drivers/gpu/drm/i915/ | wc
     58     187    5534
  git grep -w __xchg drivers/gpu/drm/i915/ | wc
     22     109    1899

and it looks like the only "logic" to this is that the converted ones
were in the "gt/" subdirectory. What a random choice, but happily it
caused a trivial conflict, and as a result I noticed how bad things
were.

Anyway, I really find this all offensively ugly and pointless. I'm not
going to pull some "fixed" version of this. This needs to go away and
never come back.

What was so magically great about the name "__xchg" that it needed to
be taken over by this function? And why was that legibly named version
of it replaced so randomly?

The *whole* point of two underscores is to say "don't use this - it's
an internal implementation". That's the historical meaning, and it's
the meaning we have in the kernel too. Two underscores means "this is
special and doesn't do everything required" (it might need locking
around it, for example).

So then making a new interface with two underscores and thinking "we
should now make random drivers use this" is fundamentally bogus.

Look, just grep for "__xchg" in the main tree (ie the one *without*
this change). It all makes sense. It's all clearly an internal helper
- as marked by that double underscore - and it's not used by any
driver or filesystem code.

Exactly like K&R and God intended.

                  Linus

Re: [GIT PULL] locking changes for v6.4

[Ingo Molnar]

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

> On Thu, Apr 27, 2023 at 12:58 PM Ingo Molnar <mingo@kernel.org> wrote:
> >
> >  - Add non-atomic __xchg() variant, use it in a couple of places
> 
> Guys, this is insane, and completely unacceptable.
> 
> I pulled this, but I'm going to unpull it, because the code is
> actively wrong and ugly.
> 
> It not only randomly decides to re-use a name that has existing users
> that now need to be fixed up.

meh - you are 100% right, I'm not sure what we were thinking there ... [ 
actually, I know what we were thinking, but it's a bit complicated - see 
the various non-perfect nomenclature options further below. ]

So the first line of our thinking was that "__" also often & additionally 
means 'lighter weight version of a similar API signature, beware, here be 
dragons, use at your own risk', and more of the focus of these particular 
changes was on identifying hand-coded xchg-ish pieces of code, such as in:

   26ace5d28d36 ("arch/*/uprobes: simplify arch_uretprobe_hijack_return_addr")

... but while that background of '__' is somewhat valid logic that we use 
quite often in various kernel facilities, it doesn't really excuse the 
sloppy decision to slap __ in front of an existing API without trying 
harder, *especially* that a better name with fetch_and_zero() already 
existed :-/

> It then *also* decides to start "preferring" this absolutely
> disgusting new name over a much more legible one in the i915 driver,
> which had this same functionality except it used a prettier name:
> 
>    fetch_and_zero()
> 
> But what then takes the cake for me is that this horribly ugly feature
> then didn't even get that right, and only randomly converted *some* of
> the users, with most of them remaining:
> 
>   git grep fetch_and_zero drivers/gpu/drm/i915/ | wc
>      58     187    5534
>   git grep -w __xchg drivers/gpu/drm/i915/ | wc
>      22     109    1899
> 
> and it looks like the only "logic" to this is that the converted ones
> were in the "gt/" subdirectory. What a random choice, but happily it
> caused a trivial conflict, and as a result I noticed how bad things
> were.
> 
> Anyway, I really find this all offensively ugly and pointless. I'm not
> going to pull some "fixed" version of this. This needs to go away and
> never come back.

Yeah. So I've rebased locking/core to take out these changes - a simple 
revert is too ugly and the history has no value here really.

Will re-send the rest of locking/core.

> What was so magically great about the name "__xchg" that it needed to be 
> taken over by this function? And why was that legibly named version of it 
> replaced so randomly?

Yeah.

So fetch_and_zero() has a bit of a nomenclature & ambiguity problem as 
well: there's already an atomic_fetch_*() API family, and it's easy to 
think that fetch_and_zero() is atomic too - a bit like how xchg() is atomic 
without mentioning 'atomic'.

Adding to the confusion is that there's already atomic APIs that don't use 
atomic_t:

  xchg()
  cmpxchg()
  try_cmpxchg()

... and by *that* implicit nomenclature logic, dropping the atomic_ from a 
atomic_fetch_and_zero() API means: 'atomic API, not using atomic_t'. Which 
fetch_and_zero() clearly isnt ...

So by all that logic and somewhat idiosynchratic API history, the new 
facility should probably not be fetch_and_zero(), but something like 
nonatomic_fetch_and_zero(), but that's quite a mouthful for something so 
simple - and the API family connection to xchg() is lost as well, which is 
a bit sad...

In all that context the least bad approach sounded to add a __ to denote 
__xchg() is 'something special and also lighter weight' (which it is).

I *think* the bigger danger in locking nomenclature is to falsely imply 
atomicity - in that sense I'm not sure fetch_and_zero() is ideal - but I 
can certainly live with it b/c the perfect name keeps eluding me.

> The *whole* point of two underscores is to say "don't use this - it's
> an internal implementation". That's the historical meaning, and it's
> the meaning we have in the kernel too. Two underscores means "this is
> special and doesn't do everything required" (it might need locking
> around it, for example).

Yeah. I do think we might want to keep one related change though:

  e27cff3d2d43 ("arch: rename all internal names __xchg to __arch_xchg")

... not because we want to use the __xchg namespace, but because an _arch 
prefix makes it even *less* likely to be used by non-infrastructure code.

> So then making a new interface with two underscores and thinking "we 
> should now make random drivers use this" is fundamentally bogus.
> 
> Look, just grep for "__xchg" in the main tree (ie the one *without* this 
> change). It all makes sense. It's all clearly an internal helper - as 
> marked by that double underscore - and it's not used by any driver or 
> filesystem code.
> 
> Exactly like K&R and God intended.

Yeah. We'll try this new facility again in v6.5, but with a better name. 
Sorry about that!

Thanks,

	Ingo

Re: [GIT PULL] locking changes for v6.4

[Andrzej Hajda]

Hi all,

Sorry for late response.
Just two cents to constructively defend myself.

On 29.04.2023 08:47, Ingo Molnar wrote:
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>> On Thu, Apr 27, 2023 at 12:58 PM Ingo Molnar <mingo@kernel.org> wrote:
>>>   - Add non-atomic __xchg() variant, use it in a couple of places
>> Guys, this is insane, and completely unacceptable.
>>
>> I pulled this, but I'm going to unpull it, because the code is
>> actively wrong and ugly.
>>
>> It not only randomly decides to re-use a name that has existing users
>> that now need to be fixed up.
> meh - you are 100% right, I'm not sure what we were thinking there ... [
> actually, I know what we were thinking, but it's a bit complicated - see
> the various non-perfect nomenclature options further below. ]
>
> So the first line of our thinking was that "__" also often & additionally
> means 'lighter weight version of a similar API signature, beware, here be
> dragons, use at your own risk', and more of the focus of these particular
> changes was on identifying hand-coded xchg-ish pieces of code, such as in:
>
>     26ace5d28d36 ("arch/*/uprobes: simplify arch_uretprobe_hijack_return_addr")
>
> ... but while that background of '__' is somewhat valid logic that we use
> quite often in various kernel facilities, it doesn't really excuse the
> sloppy decision to slap __ in front of an existing API without trying
> harder, *especially* that a better name with fetch_and_zero() already
> existed :-/
>
>> It then *also* decides to start "preferring" this absolutely
>> disgusting new name over a much more legible one in the i915 driver,
>> which had this same functionality except it used a prettier name:
>>
>>     fetch_and_zero()

My 1st approach was to follow C++ library and I have proposed 'exchange' 
[1][2].
Later I have also showed/suggested Rust's approach [3][4]:
- 'replace' for __xchg,
- 'take' for fetch_and_zero

Anyway it ended as is.

[1]: https://lore.kernel.org/lkml/Y5OFSvaYbv4XCxhE@smile.fi.intel.com/T/
[2]: https://en.cppreference.com/w/cpp/header/utility
[3]: https://doc.rust-lang.org/std/mem/index.html
[4]: 
https://lore.kernel.org/lkml/20230118153529.57695-1-andrzej.hajda@intel.com/

>>
>> But what then takes the cake for me is that this horribly ugly feature
>> then didn't even get that right, and only randomly converted *some* of
>> the users, with most of them remaining:
>>
>>    git grep fetch_and_zero drivers/gpu/drm/i915/ | wc
>>       58     187    5534
>>    git grep -w __xchg drivers/gpu/drm/i915/ | wc
>>       22     109    1899
>>
>> and it looks like the only "logic" to this is that the converted ones
>> were in the "gt/" subdirectory. What a random choice, but happily it
>> caused a trivial conflict, and as a result I noticed how bad things
>> were.

The logic was just to focus on introduction of the helper, providing few 
(not all) examples of usage and finish i915 conversion later, when the 
helper will be accepted.
Conversion should be split anyway - as i915 uses different repo for 
i915/gt, sorry for not clearly stating this.

Regards
Andrzej

>>
>> Anyway, I really find this all offensively ugly and pointless. I'm not
>> going to pull some "fixed" version of this. This needs to go away and
>> never come back.
> Yeah. So I've rebased locking/core to take out these changes - a simple
> revert is too ugly and the history has no value here really.
>
> Will re-send the rest of locking/core.
>
>> What was so magically great about the name "__xchg" that it needed to be
>> taken over by this function? And why was that legibly named version of it
>> replaced so randomly?
> Yeah.
>
> So fetch_and_zero() has a bit of a nomenclature & ambiguity problem as
> well: there's already an atomic_fetch_*() API family, and it's easy to
> think that fetch_and_zero() is atomic too - a bit like how xchg() is atomic
> without mentioning 'atomic'.
>
> Adding to the confusion is that there's already atomic APIs that don't use
> atomic_t:
>
>    xchg()
>    cmpxchg()
>    try_cmpxchg()
>
> ... and by *that* implicit nomenclature logic, dropping the atomic_ from a
> atomic_fetch_and_zero() API means: 'atomic API, not using atomic_t'. Which
> fetch_and_zero() clearly isnt ...
>
> So by all that logic and somewhat idiosynchratic API history, the new
> facility should probably not be fetch_and_zero(), but something like
> nonatomic_fetch_and_zero(), but that's quite a mouthful for something so
> simple - and the API family connection to xchg() is lost as well, which is
> a bit sad...
>
> In all that context the least bad approach sounded to add a __ to denote
> __xchg() is 'something special and also lighter weight' (which it is).
>
> I *think* the bigger danger in locking nomenclature is to falsely imply
> atomicity - in that sense I'm not sure fetch_and_zero() is ideal - but I
> can certainly live with it b/c the perfect name keeps eluding me.
>
>> The *whole* point of two underscores is to say "don't use this - it's
>> an internal implementation". That's the historical meaning, and it's
>> the meaning we have in the kernel too. Two underscores means "this is
>> special and doesn't do everything required" (it might need locking
>> around it, for example).
> Yeah. I do think we might want to keep one related change though:
>
>    e27cff3d2d43 ("arch: rename all internal names __xchg to __arch_xchg")
>
> ... not because we want to use the __xchg namespace, but because an _arch
> prefix makes it even *less* likely to be used by non-infrastructure code.
>
>> So then making a new interface with two underscores and thinking "we
>> should now make random drivers use this" is fundamentally bogus.
>>
>> Look, just grep for "__xchg" in the main tree (ie the one *without* this
>> change). It all makes sense. It's all clearly an internal helper - as
>> marked by that double underscore - and it's not used by any driver or
>> filesystem code.
>>
>> Exactly like K&R and God intended.
> Yeah. We'll try this new facility again in v6.5, but with a better name.
> Sorry about that!
>
> Thanks,
>
> 	Ingo

Re: [GIT PULL] locking changes for v6.4

[Linus Torvalds]

On Thu, May 4, 2023 at 10:30 AM Andrzej Hajda <andrzej.hajda@intel.com> wrote:
>
> Later I have also showed/suggested Rust's approach [3][4]:
> - 'replace' for __xchg,
> - 'take' for fetch_and_zero

I don't think either of those are acceptable, and both are to me
*much* worse than the existing "fetch_and_zero()" that i915 uses.

That name just makes sense and explains what it does. There is no ambiguity.

I will not be taking a pull that makes a good name worse.

Naming matters, and "use a generic function" is not an excuse for
having bad names.

              Linus