All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Remus <jremus@linux.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	bpf@vger.kernel.org, x86@kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrii Nakryiko <andrii@kernel.org>,
	Indu Bhagat <indu.bhagat@oracle.com>,
	"Jose E. Marchesi" <jemarch@gnu.org>,
	Beau Belgrave <beaub@linux.microsoft.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jens Axboe <axboe@kernel.dk>, Florian Weimer <fweimer@redhat.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>
Subject: Re: [PATCH v12 07/14] unwind_user/deferred: Make unwind deferral requests NMI-safe
Date: Wed, 2 Jul 2025 17:53:05 +0200	[thread overview]
Message-ID: <d0aa861d-e1bb-4ab4-8ccb-d9fdc39738a9@linux.ibm.com> (raw)
In-Reply-To: <20250701005451.737614486@goodmis.org>

Hello Steve!

On 01.07.2025 02:53, Steven Rostedt wrote:
> Make unwind_deferred_request() NMI-safe so tracers in NMI context can
> call it and safely request a user space stacktrace when the task exits.

> diff --git a/include/linux/unwind_deferred_types.h b/include/linux/unwind_deferred_types.h

> @@ -2,6 +2,9 @@
>  #ifndef _LINUX_UNWIND_USER_DEFERRED_TYPES_H
>  #define _LINUX_UNWIND_USER_DEFERRED_TYPES_H
>  
> +#include <asm/local64.h>
> +#include <asm/local.h>

This creates the following circular dependency, that breaks the build on
s390 as follows, whenever local64.h is included first, so that local64_t
is not yet defined when unwind_deferred_types.h gets included down the
line:


Circular dependency:

include/linux/unwind_deferred_types.h
arch/<arch>/include/generated/asm/local64.h
include/asm-generic/local64.h
include/linux/percpu.h
include/linux/sched.h
include/linux/unwind_deferred_types.h


Compile error on s390:

  CC      net/ipv4/proc.o
In file included from ./include/linux/sched.h:49,
                 from ./include/linux/percpu.h:12,
                 from ./include/asm-generic/local64.h:5,
                 from ./arch/s390/include/generated/asm/local64.h:1,
                 from ./include/linux/u64_stats_sync.h:71,
                 from ./include/net/snmp.h:47,
                 from ./include/net/netns/mib.h:5,
                 from ./include/net/net_namespace.h:17,
                 from net/ipv4/proc.c:31:
./include/linux/unwind_deferred_types.h:17:9: error: unknown type name ‘local64_t’; did you mean ‘local_t’?
   17 |         local64_t               timestamp;
      |         ^~~~~~~~~
      |         local_t


Reason it fails on s390:

net/ipv4/proc.c
+- include/net/net_namespace.h
   +- include/net/netns/mib.h
      +- include/net/snmp.h
         +- include/linux/u64_stats_sync.h
            +- arch/s390/include/generated/asm/local64.h   <-- ISSUE: local64.h gets included before unwind_deferred_types.h
               +- include/asm-generic/local64.h
                  +- include/linux/percpu.h
                     +- include/linux/sched.h
                        +- include/linux/unwind_deferred_types.h   <-- ERROR: local64_t not yet defined


Reason it does not fail on x86:

net/ipv4/proc.c
+- include/net/net_namespace.h
   +- include/linux/workqueue.h
   |  +- include/linux/timer.h
   |     +- include/linux/ktime.h
   |        +- include/linux/jiffies.h
   |           +- include/linux/time.h
   |               +- include/linux/time32.h
   |                  +- include/linux/timex.h
   |                     +- arch/x86/include/asm/timex.h
   |                        +- arch/x86/include/asm/tsc.h
   |                           +- arch/x86/include/asm/msr.h
   |                              +- include/linux/percpu.h
   |                                 +- include/linux/sched.h
   |                                    +- include/linux/unwind_deferred_types.h   <-- OK: unwind_deferred_types.h gets included before local64.h
   |                                       +- arch/x86/include/generated/asm/local64.h
   |                                          +- include/asm-generic/local64.h
   |                                           [ +- include/linux/percpu.h ]
   +- include/net/netns/mib.h
      +- include/net/snmp.h
         +- include/linux/u64_stats_sync.h
            +- arch/x86/include/generated/asm/local64.h   <-- OK: local64.h comes after unwind_deferred_types.h
             [ +- include/asm-generic/local64.h ]
                [ +- include/linux/percpu.h ]
                   [ +- include/linux/sched.h ]
                      [ +- include/linux/unwind_deferred_types.h ]


My colleague Heiko Carstens (on Cc) suggested the following potential fix
that breaks the circular dependency, provided local[64].h does not ever
need percpu.h.  At least I verified that defconfig x86 and s390 builds
still work.

diff --git a/include/asm-generic/local.h b/include/asm-generic/local.h
index 7f97018df66f..3e7ce6a9e18e 100644
--- a/include/asm-generic/local.h
+++ b/include/asm-generic/local.h
@@ -2,7 +2,6 @@
 #ifndef _ASM_GENERIC_LOCAL_H
 #define _ASM_GENERIC_LOCAL_H
 
-#include <linux/percpu.h>
 #include <linux/atomic.h>
 #include <asm/types.h>
 
diff --git a/include/asm-generic/local64.h b/include/asm-generic/local64.h
index 14963a7a6253..1f9af89916cb 100644
--- a/include/asm-generic/local64.h
+++ b/include/asm-generic/local64.h
@@ -2,7 +2,6 @@
 #ifndef _ASM_GENERIC_LOCAL64_H
 #define _ASM_GENERIC_LOCAL64_H
 
-#include <linux/percpu.h>
 #include <asm/types.h>

> +
>  struct unwind_cache {
>  	unsigned int		nr_entries;
>  	unsigned long		entries[];
> @@ -10,8 +13,8 @@ struct unwind_cache {
>  struct unwind_task_info {
>  	struct unwind_cache	*cache;
>  	struct callback_head	work;
> -	u64			timestamp;
> -	int			pending;
> +	local64_t		timestamp;
> +	local_t			pending;
>  };
>  
>  #endif /* _LINUX_UNWIND_USER_DEFERRED_TYPES_H */

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/


  reply	other threads:[~2025-07-02 15:54 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-01  0:53 [PATCH v12 00/14] unwind_user: x86: Deferred unwinding infrastructure Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 01/14] unwind_user: Add user space unwinding API Steven Rostedt
2025-07-04 17:58   ` Mathieu Desnoyers
2025-07-04 18:20   ` Mathieu Desnoyers
2025-07-07 19:42     ` Steven Rostedt
2025-07-07 21:01       ` Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 02/14] unwind_user: Add frame pointer support Steven Rostedt
2025-07-01  2:10   ` Linus Torvalds
2025-07-01  2:56     ` Steven Rostedt
2025-07-01  3:05       ` Steven Rostedt
2025-07-01 15:36       ` Jens Remus
2025-07-02 23:50         ` Steven Rostedt
2025-07-03 16:21           ` Jens Remus
2025-07-07 21:28             ` Steven Rostedt
2025-07-01  4:46     ` Florian Weimer
2025-07-01 12:14       ` Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 03/14] unwind_user: Add compat mode " Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 04/14] unwind_user/deferred: Add unwind_user_faultable() Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 05/14] unwind_user/deferred: Add unwind cache Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 06/14] unwind_user/deferred: Add deferred unwinding interface Steven Rostedt
2025-07-02 16:36   ` Peter Zijlstra
2025-07-02 16:42     ` Steven Rostedt
2025-07-02 16:56       ` Linus Torvalds
2025-07-02 17:26         ` Steven Rostedt
2025-07-02 17:48           ` Steven Rostedt
2025-07-02 18:21             ` Linus Torvalds
2025-07-02 18:47               ` Mathieu Desnoyers
2025-07-02 19:05                 ` Steven Rostedt
2025-07-02 19:12                   ` Mathieu Desnoyers
2025-07-02 19:21                     ` Steven Rostedt
2025-07-02 19:36                       ` Steven Rostedt
2025-07-02 19:40                         ` Steven Rostedt
2025-07-02 19:48                           ` Mathieu Desnoyers
2025-07-02 20:10                             ` Steven Rostedt
2025-07-02 19:43                       ` Mathieu Desnoyers
2025-07-02 19:51                         ` Steven Rostedt
2025-07-02 18:57               ` Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 07/14] unwind_user/deferred: Make unwind deferral requests NMI-safe Steven Rostedt
2025-07-02 15:53   ` Jens Remus [this message]
2025-07-02 19:11     ` Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 08/14] unwind deferred: Use bitmask to determine which callbacks to call Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 09/14] unwind deferred: Use SRCU unwind_deferred_task_work() Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 10/14] unwind: Clear unwind_mask on exit back to user space Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 11/14] unwind: Add USED bit to only have one conditional on way " Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 12/14] unwind: Finish up unwind when a task exits Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 13/14] unwind_user/x86: Enable frame pointer unwinding on x86 Steven Rostedt
2025-07-01  0:53 ` [PATCH v12 14/14] unwind_user/x86: Enable compat mode " Steven Rostedt
2025-07-01  2:06 ` [PATCH v12 00/14] unwind_user: x86: Deferred unwinding infrastructure Linus Torvalds
2025-07-01  2:45   ` Steven Rostedt
2025-07-01 22:49     ` Kees Cook
2025-07-01 23:26       ` Steven Rostedt
2025-07-02 14:56         ` Kees Cook
2025-07-02 16:20           ` Mathieu Desnoyers

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=d0aa861d-e1bb-4ab4-8ccb-d9fdc39738a9@linux.ibm.com \
    --to=jremus@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrii@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=beaub@linux.microsoft.com \
    --cc=bpf@vger.kernel.org \
    --cc=fweimer@redhat.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=indu.bhagat@oracle.com \
    --cc=jemarch@gnu.org \
    --cc=jolsa@kernel.org \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.