Re: [Intel-gfx] [PATCH v5 0/7] Introduce __xchg, non-atomic xchg
On 05.04.2024 16:47, Jani Nikula wrote: On Mon, 27 Feb 2023, Peter Zijlstra wrote: On Thu, Feb 23, 2023 at 10:24:19PM +0100, Andrzej Hajda wrote: On 22.02.2023 18:04, Peter Zijlstra wrote: On Wed, Jan 18, 2023 at 04:35:22PM +0100, Andrzej Hajda wrote: Andrzej Hajda (7): 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 io_uring: use __xchg if possible qed: use __xchg if possible drm/i915/gt: use __xchg instead of internal helper Nothing crazy in here I suppose, I somewhat wonder why you went through the trouble, but meh. If you are asking why I have proposed this patchset, then the answer is simple, 1st I've tried to find a way to move internal i915 helper to core (see patch 7). Then I was looking for possible other users of this helper. And apparently there are many of them, patches 3-7 shows some. You want me to take this through te locking tree (for the next cycle, not this one) where I normally take atomic things or does someone else want this? If you could take it I will be happy. OK, I'll go queue it in tip/locking/core after -rc1. Thanks! Is this where the series fell between the cracks, or was there some follow-up that I missed? I think this would still be useful. Andrzej, would you mind rebasing and resending if there are no objections? The patchset was rejected/dropped by Linus at the pull-request stage. He didn't like many things, but the most __xchg name. However he was quite positive about i915 name fetch_and_zero. I can try to revive patchset with fetch_and_zero, and maybe fetch_and_set, instead of __xchg. Regards Andrzej BR, Jani.
Re: [Intel-gfx] [PATCH v5 0/7] Introduce __xchg, non-atomic xchg
On Mon, 27 Feb 2023, Peter Zijlstra wrote: > On Thu, Feb 23, 2023 at 10:24:19PM +0100, Andrzej Hajda wrote: >> On 22.02.2023 18:04, Peter Zijlstra wrote: >> > On Wed, Jan 18, 2023 at 04:35:22PM +0100, Andrzej Hajda wrote: >> > >> > > Andrzej Hajda (7): >> > >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 >> > >io_uring: use __xchg if possible >> > >qed: use __xchg if possible >> > >drm/i915/gt: use __xchg instead of internal helper >> > >> > Nothing crazy in here I suppose, I somewhat wonder why you went through >> > the trouble, but meh. >> >> If you are asking why I have proposed this patchset, then the answer is >> simple, 1st I've tried to find a way to move internal i915 helper to core >> (see patch 7). >> Then I was looking for possible other users of this helper. And apparently >> there are many of them, patches 3-7 shows some. >> >> >> > >> > You want me to take this through te locking tree (for the next cycle, >> > not this one) where I normally take atomic things or does someone else >> > want this? >> >> If you could take it I will be happy. > > OK, I'll go queue it in tip/locking/core after -rc1. Thanks! Is this where the series fell between the cracks, or was there some follow-up that I missed? I think this would still be useful. Andrzej, would you mind rebasing and resending if there are no objections? BR, Jani. -- Jani Nikula, Intel
Re: [Intel-gfx] [PATCH v5 0/7] Introduce __xchg, non-atomic xchg
On Thu, Feb 23, 2023 at 10:24:19PM +0100, Andrzej Hajda wrote: > On 22.02.2023 18:04, Peter Zijlstra wrote: > > On Wed, Jan 18, 2023 at 04:35:22PM +0100, Andrzej Hajda wrote: > > > > > Andrzej Hajda (7): > > >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 > > >io_uring: use __xchg if possible > > >qed: use __xchg if possible > > >drm/i915/gt: use __xchg instead of internal helper > > > > Nothing crazy in here I suppose, I somewhat wonder why you went through > > the trouble, but meh. > > If you are asking why I have proposed this patchset, then the answer is > simple, 1st I've tried to find a way to move internal i915 helper to core > (see patch 7). > Then I was looking for possible other users of this helper. And apparently > there are many of them, patches 3-7 shows some. > > > > > > You want me to take this through te locking tree (for the next cycle, > > not this one) where I normally take atomic things or does someone else > > want this? > > If you could take it I will be happy. OK, I'll go queue it in tip/locking/core after -rc1. Thanks!
Re: [Intel-gfx] [PATCH v5 0/7] Introduce __xchg, non-atomic xchg
On 22.02.2023 18:04, Peter Zijlstra wrote: On Wed, Jan 18, 2023 at 04:35:22PM +0100, Andrzej Hajda wrote: Andrzej Hajda (7): 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 io_uring: use __xchg if possible qed: use __xchg if possible drm/i915/gt: use __xchg instead of internal helper Nothing crazy in here I suppose, I somewhat wonder why you went through the trouble, but meh. If you are asking why I have proposed this patchset, then the answer is simple, 1st I've tried to find a way to move internal i915 helper to core (see patch 7). Then I was looking for possible other users of this helper. And apparently there are many of them, patches 3-7 shows some. You want me to take this through te locking tree (for the next cycle, not this one) where I normally take atomic things or does someone else want this? If you could take it I will be happy. Regards Andrzej
Re: [Intel-gfx] [PATCH v5 0/7] Introduce __xchg, non-atomic xchg
Hi, Ping on the series. Arnd, Andrew is there anything more I can do to push the process forward? Regards Andrzej On 18.01.2023 16:35, Andrzej Hajda wrote: Hi all, The helper is tiny and there are advices we can live without it, so I want to present few arguments why it would be good to have it: 1. Code readability/simplification/number of lines: - decreases number of lines, - it often eliminates local variables, - for real examples see patches 3+. 2. Presence of similar helpers in other somehow related languages/libs: a) Rust[1]: 'replace' from std::mem module, there is also 'take' helper (__xchg(, 0)), which is the same as private helper in i915 - fetch_and_zero, see latest patch. b) C++ [2]: 'exchange' from utility header. If the idea is OK there are still 2 questions to answer: 1. Name of the helper, __xchg follows kernel conventions, but for me Rust names are also OK. 2. Where to put the helper: a) as in this patchset include/linux/non-atomic/xchg.h, proposed by Andy Shevchenko, b) include/linux/utils.h ? any better name? Some kind of container for simple helpers. All __xchg conversions were performed using cocci script, then manually adjusted if necessary. There is lot of places it can be used in, I have just chosen some of them. I can provide cocci script to detect others (not all), if necessary. Changes: v2: squashed all __xchg -> __arch_xchg t one patch (Arnd) v3: fixed alpha/xchg_local (l...@intel.com) v4: adjusted indentation (Heiko) v5: added more __xchg conversions - patches 3-6, added tags [1]: https://doc.rust-lang.org/std/mem/index.html [2]: https://en.cppreference.com/w/cpp/header/utility Regards Andrzej Andrzej Hajda (7): 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 io_uring: use __xchg if possible qed: use __xchg if possible drm/i915/gt: use __xchg instead of internal helper arch/alpha/include/asm/cmpxchg.h | 10 +- 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/m68k/include/asm/cmpxchg.h | 6 +++--- arch/mips/include/asm/cmpxchg.h | 4 ++-- 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/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/xtensa/include/asm/cmpxchg.h | 4 ++-- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- .../gpu/drm/i915/gt/intel_engine_heartbeat.c | 4 ++-- .../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 +- .../drm/i915/gt/selftest_ring_submission.c| 2 +- drivers/gpu/drm/i915/gt/selftest_timeline.c | 2 +- drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.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/linux/llist.h | 6 ++ include/linux/non-atomic/xchg.h | 19 +++ include/linux/qed/qed_chain.h | 19 +++ io_uring/io_uring.c