public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Sean Christopherson <seanjc@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	Boqun Feng <boqun@kernel.org>, Waiman Long <longman@redhat.com>,
	linux-kernel@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
	Steven Rostedt <rostedt@goodmis.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Kees Cook <kees@kernel.org>, Jann Horn <jannh@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org
Subject: Re: [PATCH 01/62] kvm: Make pi_enable_wakeup_handler() easier to analyze
Date: Thu, 19 Mar 2026 00:31:15 +0100	[thread overview]
Message-ID: <abs1w28YCs8Qj71w@elver.google.com> (raw)
In-Reply-To: <e3946223-4543-4a76-a328-9c6865e95192@acm.org>

On Thu, Feb 26, 2026 at 04:19PM -0800, Bart Van Assche wrote:
> On 2/26/26 12:13 PM, Marco Elver wrote:
> > The goal of RELOC_HIDE is to make the optimizer be less aggressive.
> > But the Thread Safety Analysis's alias analysis happens during
> > semantic analysis and is completely detached from the optimizer, and
> > we could potentially construct an expression that (a) lets Thread
> > Safety Analysis figure out that __ptr is an alias to ptr, while (b)
> > still hiding it from the optimizer. But I think we're sufficiently
> > scared of breaking (b) that I'm not sure if this is feasible in a
> > clean enough way that won't have other side-effects (e.g. worse
> > codegen).
> 
> Does the thread-safety alias analyzer assume that function calls with
> identical inputs produce identical outputs? If so, how about changing
> RELOC_HIDE() from a macro into an inline function? Would that be
> sufficient to make the thread-safety checker recognize identical
> per_cpu() expressions as identical without affecting the behavior of the
> optimizer?

The below works:

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index af16624b29fd..cb2f6050bdf7 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -149,10 +149,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 #endif
 
 #ifndef RELOC_HIDE
-# define RELOC_HIDE(ptr, off)					\
-  ({ unsigned long __ptr;					\
-     __ptr = (unsigned long) (ptr);				\
-    (typeof(ptr)) (__ptr + (off)); })
+# define RELOC_HIDE(ptr, off) ((typeof(ptr))((unsigned long)(ptr) + (off)))
 #endif
 
 #define absolute_pointer(val)	RELOC_HIDE((void *)(val), 0)

That preserves original RELOC_HIDE intent (hide likely out-of-bounds
pointer calculation via unsigned long cast) - GCC has its own version of
RELOC_HIDE, so this seems to be exclusively for Clang.  For codegen, the
temporary variable was just optimized away, so I'm not sure what benefit
that indirection had at all. So all in all that should be equivalent to
before, and looks a lot cleaner.

The reason it works for Thread Safety Analysis is that TSA's alias
analysis strips away inner casts when resolving pointer aliases. Whereas
if there was an intermediate non-pointer (here: ulong) variable, it
stops.

Unless there are concerns, I'll send that as a patch.

  reply	other threads:[~2026-03-18 23:31 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-23 21:50 [PATCH 00/62] Bug fixes and refactoring patches related to locking Bart Van Assche
2026-02-23 21:50 ` [PATCH 01/62] kvm: Make pi_enable_wakeup_handler() easier to analyze Bart Van Assche
2026-02-24 18:20   ` Sean Christopherson
2026-02-24 19:25     ` Bart Van Assche
2026-02-26 17:47       ` Sean Christopherson
2026-02-26 20:13         ` Marco Elver
2026-02-27  0:19           ` Bart Van Assche
2026-03-18 23:31             ` Marco Elver [this message]
2026-03-19 14:43               ` Marco Elver
2026-02-26 22:36         ` Bart Van Assche
2026-02-26 22:41           ` Sean Christopherson
2026-02-23 21:50 ` [PATCH 02/62] blk-ioc: Prepare for enabling thread-safety analysis Bart Van Assche
2026-02-23 21:50 ` [PATCH 03/62] drbd: Balance RCU calls in drbd_adm_dump_devices() Bart Van Assche
2026-02-23 21:50 ` [PATCH 04/62] dax/bus.c: Fix a locking bug Bart Van Assche
2026-02-23 21:50 ` [PATCH 05/62] dma-buf: Convert dma_buf_import_sync_file() to the early-return style Bart Van Assche
2026-02-23 21:50 ` [PATCH 06/62] dma-buf: Handle all dma_resv_lock() errors Bart Van Assche
2026-02-23 21:50 ` [PATCH 07/62] drm/amdgpu: Unlock a mutex before destroying it Bart Van Assche
2026-02-24  8:26   ` Christian König
2026-02-23 21:50 ` [PATCH 08/62] drm/amdgpu: Fix locking bugs in error paths Bart Van Assche
2026-02-24  8:28   ` Christian König
2026-02-24 14:32     ` Alex Deucher
2026-02-23 21:50 ` [PATCH 09/62] drm: bridge: cdns-mhdp8546: Fix a locking bug in an error path Bart Van Assche
2026-02-23 21:50 ` [PATCH 10/62] drm: Make drm_read() easier to analyze Bart Van Assche
2026-02-23 21:50 ` [PATCH 11/62] drm/pagemap: Unlock cache->lock before freeing it Bart Van Assche
2026-02-23 21:50 ` [PATCH 12/62] drm/gpusvm.c: Fix a locking bug in an error path Bart Van Assche
2026-02-23 21:50 ` [PATCH 13/62] drm/qxl: Fix a buffer leak " Bart Van Assche
2026-02-23 21:50 ` [PATCH 14/62] hwmon: (it87) Check the it87_lock() return value Bart Van Assche
2026-02-23 21:50 ` [PATCH 15/62] Input: synaptics-rmi4 - fix a locking bug in an error path Bart Van Assche
2026-02-23 21:58   ` Dmitry Torokhov
2026-02-23 22:05     ` Bart Van Assche
2026-02-23 21:50 ` [PATCH 16/62] md: Make mddev_suspend() easier to analyze Bart Van Assche
2026-02-23 21:50 ` [PATCH 17/62] bnxt_en: Make bnxt_resume() " Bart Van Assche
2026-02-23 21:50 ` [PATCH 18/62] bnxt_en: Fix bnxt_dl_reload_up() Bart Van Assche
2026-02-23 21:50 ` [PATCH 19/62] ice: Fix a locking bug in an error path Bart Van Assche
2026-02-23 22:01 ` [PATCH 00/62] Bug fixes and refactoring patches related to locking Peter Zijlstra
2026-02-23 22:13   ` Bart Van Assche
  -- strict thread matches above, loose matches on Subject: below --
2026-02-23 22:00 Bart Van Assche
2026-02-23 22:00 ` [PATCH 01/62] kvm: Make pi_enable_wakeup_handler() easier to analyze Bart Van Assche

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=abs1w28YCs8Qj71w@elver.google.com \
    --to=elver@google.com \
    --cc=boqun@kernel.org \
    --cc=bvanassche@acm.org \
    --cc=hch@lst.de \
    --cc=jannh@google.com \
    --cc=kees@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=seanjc@google.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox