public inbox for linux-perf-users@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] perf record: Add support for arch_sdt_arg_parse_op() on s390
@ 2026-03-17 11:06 Thomas Richter
  2026-03-17 15:32 ` Ian Rogers
  2026-03-18  1:52 ` Mi, Dapeng
  0 siblings, 2 replies; 5+ messages in thread
From: Thomas Richter @ 2026-03-17 11:06 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linux-perf-users, acme, namhyung,
	irogers, dapeng1.mi
  Cc: agordeev, gor, sumanthk, hca, japo, Thomas Richter

commit e5e66adfe45a6 ("perf regs: Remove __weak attributive arch_sdt_arg_parse_op() function")
removes arch_sdt_arg_parse_op() functions and s390 support is lost.
The following warning is printed:

  Unknown ELF machine 22, standard arguments parse will be skipped.

ELF machine 22 is the EM_S390 host. This happens with command
  # ./perf record -v -- stress-ng -t 1s --matrix 0
on a z/VM system when the event is not specified.

Add s390 specific __perf_sdt_arg_parse_op_s390() function to support
-architecture calls to arch_sdt_arg_parse_op() for s390.
The warning disappears.

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>
Tested-by: Jan Polensky <japo@linux.ibm.com>
---
 .../perf/util/perf-regs-arch/perf_regs_s390.c | 89 +++++++++++++++++++
 tools/perf/util/perf_regs.c                   |  3 +
 tools/perf/util/perf_regs.h                   |  1 +
 3 files changed, 93 insertions(+)

diff --git a/tools/perf/util/perf-regs-arch/perf_regs_s390.c b/tools/perf/util/perf-regs-arch/perf_regs_s390.c
index c61df24edf0f..2aa70eb23311 100644
--- a/tools/perf/util/perf-regs-arch/perf_regs_s390.c
+++ b/tools/perf/util/perf-regs-arch/perf_regs_s390.c
@@ -1,7 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <errno.h>
+#include <regex.h>
 #include "../perf_regs.h"
 #include "../../arch/s390/include/perf_regs.h"
+#include "debug.h"
+
+#include <linux/zalloc.h>
+#include <linux/kernel.h>
 
 uint64_t __perf_reg_mask_s390(bool intr __maybe_unused)
 {
@@ -95,3 +101,86 @@ uint64_t __perf_reg_sp_s390(void)
 {
 	return PERF_REG_S390_R15;
 }
+
+/* %rXX */
+#define SDT_OP_REGEX1  "^%(r([0-9]|1[0-5]))$"
+/* -###(%rXX) */
+#define SDT_OP_REGEX2  "^([+-]?[0-9]+)\\(%(r[0-9]|r1[0-5])\\)$"
+static regex_t sdt_op_regex1, sdt_op_regex2;
+
+static int sdt_init_op_regex(void)
+{
+	static int initialized;
+	int ret = 0;
+
+	if (initialized)
+		return 0;
+
+	ret = regcomp(&sdt_op_regex1, SDT_OP_REGEX1, REG_EXTENDED);
+	if (ret)
+		goto error;
+	initialized = 1;
+
+	ret = regcomp(&sdt_op_regex2, SDT_OP_REGEX2, REG_EXTENDED);
+	if (ret)
+		goto free_regex1;
+	initialized = 2;
+
+	return 0;
+
+free_regex1:
+	regfree(&sdt_op_regex1);
+error:
+	pr_debug4("Regex compilation error, initialized %d\n", initialized);
+	return ret;
+}
+
+/*
+ * Parse OP and convert it into uprobe format, which is, +/-NUM(%gprREG).
+ * Possible variants of OP are:
+ *	Format		Example
+ *	-------------------------
+ *	NUM(%rREG)	48(%r1)
+ *	-NUM(%rREG)	-48(%r1)
+ *	%rREG		%r1
+ */
+int __perf_sdt_arg_parse_op_s390(char *old_op, char **new_op)
+{
+	int ret, new_len;
+	regmatch_t rm[6];
+	unsigned long i;
+
+	*new_op = NULL;
+	ret = sdt_init_op_regex();
+	if (ret)
+		return -EINVAL;
+
+	if (!regexec(&sdt_op_regex1, old_op, 3, rm, 0)) {
+		/* Extract %rX */
+		new_len = 2;    /* % NULL */
+		new_len += (int)(rm[1].rm_eo - rm[1].rm_so);
+		*new_op = zalloc(new_len);
+		if (!*new_op)
+			return -ENOMEM;
+
+		scnprintf(*new_op, new_len, "%.*s",
+			  (int)(rm[1].rm_eo - rm[1].rm_so), old_op + rm[1].rm_so);
+	} else if (!regexec(&sdt_op_regex2, old_op, ARRAY_SIZE(rm), rm, 0)) {
+		/* Extract #(%rX) */
+		new_len = 4;    /* (%)NULL */
+		for (i = 1; i < ARRAY_SIZE(rm) && rm[i].rm_so != -1; ++i)
+			new_len += (int)(rm[i].rm_eo - rm[i].rm_so);
+		*new_op = zalloc(new_len);
+		if (!*new_op)
+			return -ENOMEM;
+
+		scnprintf(*new_op, new_len, "%.*s(%.*s)",
+			  (int)(rm[1].rm_eo - rm[1].rm_so), old_op + rm[1].rm_so,
+			  (int)(rm[2].rm_eo - rm[2].rm_so), old_op + rm[2].rm_so);
+	} else {
+		pr_debug4("Skipping unsupported SDT argument: %s\n", old_op);
+		return SDT_ARG_SKIP;
+	}
+
+	return SDT_ARG_VALID;
+}
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index 5b8f34beb24e..f52b0e1f7fc7 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -23,6 +23,9 @@ int perf_sdt_arg_parse_op(uint16_t e_machine, char *old_op, char **new_op)
 	case EM_X86_64:
 		ret = __perf_sdt_arg_parse_op_x86(old_op, new_op);
 		break;
+	case EM_S390:
+		ret = __perf_sdt_arg_parse_op_s390(old_op, new_op);
+		break;
 	default:
 		pr_debug("Unknown ELF machine %d, standard arguments parse will be skipped.\n",
 			 e_machine);
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 7c04700bf837..573f0d1dfe04 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -62,6 +62,7 @@ uint64_t __perf_reg_mask_s390(bool intr);
 const char *__perf_reg_name_s390(int id);
 uint64_t __perf_reg_ip_s390(void);
 uint64_t __perf_reg_sp_s390(void);
+int __perf_sdt_arg_parse_op_s390(char *old_op, char **new_op);
 
 int __perf_sdt_arg_parse_op_x86(char *old_op, char **new_op);
 uint64_t __perf_reg_mask_x86(bool intr);
-- 
2.53.0


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

* Re: [PATCH v3] perf record: Add support for arch_sdt_arg_parse_op() on s390
  2026-03-17 11:06 [PATCH v3] perf record: Add support for arch_sdt_arg_parse_op() on s390 Thomas Richter
@ 2026-03-17 15:32 ` Ian Rogers
  2026-03-18  1:52 ` Mi, Dapeng
  1 sibling, 0 replies; 5+ messages in thread
From: Ian Rogers @ 2026-03-17 15:32 UTC (permalink / raw)
  To: Thomas Richter
  Cc: linux-kernel, linux-s390, linux-perf-users, acme, namhyung,
	dapeng1.mi, agordeev, gor, sumanthk, hca, japo

On Tue, Mar 17, 2026 at 4:06 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>
> commit e5e66adfe45a6 ("perf regs: Remove __weak attributive arch_sdt_arg_parse_op() function")
> removes arch_sdt_arg_parse_op() functions and s390 support is lost.
> The following warning is printed:

Apologies again for this breakage.

>   Unknown ELF machine 22, standard arguments parse will be skipped.
>
> ELF machine 22 is the EM_S390 host. This happens with command
>   # ./perf record -v -- stress-ng -t 1s --matrix 0
> on a z/VM system when the event is not specified.
>
> Add s390 specific __perf_sdt_arg_parse_op_s390() function to support
> -architecture calls to arch_sdt_arg_parse_op() for s390.
> The warning disappears.
>
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Tested-by: Jan Polensky <japo@linux.ibm.com>
> ---
>  .../perf/util/perf-regs-arch/perf_regs_s390.c | 89 +++++++++++++++++++
>  tools/perf/util/perf_regs.c                   |  3 +
>  tools/perf/util/perf_regs.h                   |  1 +
>  3 files changed, 93 insertions(+)
>
> diff --git a/tools/perf/util/perf-regs-arch/perf_regs_s390.c b/tools/perf/util/perf-regs-arch/perf_regs_s390.c
> index c61df24edf0f..2aa70eb23311 100644
> --- a/tools/perf/util/perf-regs-arch/perf_regs_s390.c
> +++ b/tools/perf/util/perf-regs-arch/perf_regs_s390.c
> @@ -1,7 +1,13 @@
>  // SPDX-License-Identifier: GPL-2.0
>
> +#include <errno.h>
> +#include <regex.h>
>  #include "../perf_regs.h"
>  #include "../../arch/s390/include/perf_regs.h"
> +#include "debug.h"
> +
> +#include <linux/zalloc.h>
> +#include <linux/kernel.h>
>
>  uint64_t __perf_reg_mask_s390(bool intr __maybe_unused)
>  {
> @@ -95,3 +101,86 @@ uint64_t __perf_reg_sp_s390(void)
>  {
>         return PERF_REG_S390_R15;
>  }
> +
> +/* %rXX */
> +#define SDT_OP_REGEX1  "^%(r([0-9]|1[0-5]))$"
> +/* -###(%rXX) */
> +#define SDT_OP_REGEX2  "^([+-]?[0-9]+)\\(%(r[0-9]|r1[0-5])\\)$"
> +static regex_t sdt_op_regex1, sdt_op_regex2;
> +
> +static int sdt_init_op_regex(void)
> +{
> +       static int initialized;
> +       int ret = 0;
> +
> +       if (initialized)
> +               return 0;
> +
> +       ret = regcomp(&sdt_op_regex1, SDT_OP_REGEX1, REG_EXTENDED);
> +       if (ret)
> +               goto error;
> +       initialized = 1;
> +
> +       ret = regcomp(&sdt_op_regex2, SDT_OP_REGEX2, REG_EXTENDED);
> +       if (ret)
> +               goto free_regex1;

Sashiko's review:
https://sashiko.dev/#/patchset/20260317110641.39975-1-tmricht%40linux.ibm.com
notes that here 'initialized' will still be 1, should it be made 0 again?

It triggers an ENOMEM, so I doubt this will be very useful, but it may
make the code read better.

> +       initialized = 2;
> +
> +       return 0;
> +
> +free_regex1:
> +       regfree(&sdt_op_regex1);
> +error:
> +       pr_debug4("Regex compilation error, initialized %d\n", initialized);
> +       return ret;
> +}
> +
> +/*
> + * Parse OP and convert it into uprobe format, which is, +/-NUM(%gprREG).
> + * Possible variants of OP are:
> + *     Format          Example
> + *     -------------------------
> + *     NUM(%rREG)      48(%r1)
> + *     -NUM(%rREG)     -48(%r1)
> + *     %rREG           %r1
> + */
> +int __perf_sdt_arg_parse_op_s390(char *old_op, char **new_op)
> +{
> +       int ret, new_len;
> +       regmatch_t rm[6];
> +       unsigned long i;
> +
> +       *new_op = NULL;
> +       ret = sdt_init_op_regex();
> +       if (ret)
> +               return -EINVAL;
> +
> +       if (!regexec(&sdt_op_regex1, old_op, 3, rm, 0)) {
> +               /* Extract %rX */
> +               new_len = 2;    /* % NULL */
> +               new_len += (int)(rm[1].rm_eo - rm[1].rm_so);
> +               *new_op = zalloc(new_len);
> +               if (!*new_op)
> +                       return -ENOMEM;
> +
> +               scnprintf(*new_op, new_len, "%.*s",
> +                         (int)(rm[1].rm_eo - rm[1].rm_so), old_op + rm[1].rm_so);

Sashiko notes this is probably more of an issue:

The allocation size accounts for a % character, but the scnprintf format
string "%.*s" seems to omit it.

Will this output r1 instead of %r1?

> +       } else if (!regexec(&sdt_op_regex2, old_op, ARRAY_SIZE(rm), rm, 0)) {
> +               /* Extract #(%rX) */
> +               new_len = 4;    /* (%)NULL */
> +               for (i = 1; i < ARRAY_SIZE(rm) && rm[i].rm_so != -1; ++i)
> +                       new_len += (int)(rm[i].rm_eo - rm[i].rm_so);
> +               *new_op = zalloc(new_len);
> +               if (!*new_op)
> +                       return -ENOMEM;
> +
> +               scnprintf(*new_op, new_len, "%.*s(%.*s)",
> +                         (int)(rm[1].rm_eo - rm[1].rm_so), old_op + rm[1].rm_so,
> +                         (int)(rm[2].rm_eo - rm[2].rm_so), old_op + rm[2].rm_so);


Similarly, does this scnprintf format string "%.*s(%.*s)" need a %% character
to produce the correct uprobe format? As written, it appears it will output
48(r1) instead of 48(%r1).

Thanks,
Ian

> +       } else {
> +               pr_debug4("Skipping unsupported SDT argument: %s\n", old_op);
> +               return SDT_ARG_SKIP;
> +       }
> +
> +       return SDT_ARG_VALID;
> +}
> diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
> index 5b8f34beb24e..f52b0e1f7fc7 100644
> --- a/tools/perf/util/perf_regs.c
> +++ b/tools/perf/util/perf_regs.c
> @@ -23,6 +23,9 @@ int perf_sdt_arg_parse_op(uint16_t e_machine, char *old_op, char **new_op)
>         case EM_X86_64:
>                 ret = __perf_sdt_arg_parse_op_x86(old_op, new_op);
>                 break;
> +       case EM_S390:
> +               ret = __perf_sdt_arg_parse_op_s390(old_op, new_op);
> +               break;
>         default:
>                 pr_debug("Unknown ELF machine %d, standard arguments parse will be skipped.\n",
>                          e_machine);
> diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> index 7c04700bf837..573f0d1dfe04 100644
> --- a/tools/perf/util/perf_regs.h
> +++ b/tools/perf/util/perf_regs.h
> @@ -62,6 +62,7 @@ uint64_t __perf_reg_mask_s390(bool intr);
>  const char *__perf_reg_name_s390(int id);
>  uint64_t __perf_reg_ip_s390(void);
>  uint64_t __perf_reg_sp_s390(void);
> +int __perf_sdt_arg_parse_op_s390(char *old_op, char **new_op);
>
>  int __perf_sdt_arg_parse_op_x86(char *old_op, char **new_op);
>  uint64_t __perf_reg_mask_x86(bool intr);
> --
> 2.53.0
>

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

* Re: [PATCH v3] perf record: Add support for arch_sdt_arg_parse_op() on s390
  2026-03-17 11:06 [PATCH v3] perf record: Add support for arch_sdt_arg_parse_op() on s390 Thomas Richter
  2026-03-17 15:32 ` Ian Rogers
@ 2026-03-18  1:52 ` Mi, Dapeng
  2026-03-18  6:27   ` Thomas Richter
  1 sibling, 1 reply; 5+ messages in thread
From: Mi, Dapeng @ 2026-03-18  1:52 UTC (permalink / raw)
  To: Thomas Richter, linux-kernel, linux-s390, linux-perf-users, acme,
	namhyung, irogers
  Cc: agordeev, gor, sumanthk, hca, japo


On 3/17/2026 7:06 PM, Thomas Richter wrote:
> commit e5e66adfe45a6 ("perf regs: Remove __weak attributive arch_sdt_arg_parse_op() function")
> removes arch_sdt_arg_parse_op() functions and s390 support is lost.
> The following warning is printed:

Not sure if I miss something, but it looks there was also no s390 specific
support for arch_sdt_arg_parse_op() before the commit e5e66adfe45a6 ("perf
regs: Remove __weak attributive arch_sdt_arg_parse_op() function") and we
would see same warning even without the commit e5e66adfe45a6, right?


>
>   Unknown ELF machine 22, standard arguments parse will be skipped.
>
> ELF machine 22 is the EM_S390 host. This happens with command
>   # ./perf record -v -- stress-ng -t 1s --matrix 0
> on a z/VM system when the event is not specified.
>
> Add s390 specific __perf_sdt_arg_parse_op_s390() function to support
> -architecture calls to arch_sdt_arg_parse_op() for s390.
> The warning disappears.
>
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Cc: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Tested-by: Jan Polensky <japo@linux.ibm.com>
> ---
>  .../perf/util/perf-regs-arch/perf_regs_s390.c | 89 +++++++++++++++++++
>  tools/perf/util/perf_regs.c                   |  3 +
>  tools/perf/util/perf_regs.h                   |  1 +
>  3 files changed, 93 insertions(+)
>
> diff --git a/tools/perf/util/perf-regs-arch/perf_regs_s390.c b/tools/perf/util/perf-regs-arch/perf_regs_s390.c
> index c61df24edf0f..2aa70eb23311 100644
> --- a/tools/perf/util/perf-regs-arch/perf_regs_s390.c
> +++ b/tools/perf/util/perf-regs-arch/perf_regs_s390.c
> @@ -1,7 +1,13 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> +#include <errno.h>
> +#include <regex.h>
>  #include "../perf_regs.h"
>  #include "../../arch/s390/include/perf_regs.h"
> +#include "debug.h"
> +
> +#include <linux/zalloc.h>
> +#include <linux/kernel.h>
>  
>  uint64_t __perf_reg_mask_s390(bool intr __maybe_unused)
>  {
> @@ -95,3 +101,86 @@ uint64_t __perf_reg_sp_s390(void)
>  {
>  	return PERF_REG_S390_R15;
>  }
> +
> +/* %rXX */
> +#define SDT_OP_REGEX1  "^%(r([0-9]|1[0-5]))$"
> +/* -###(%rXX) */
> +#define SDT_OP_REGEX2  "^([+-]?[0-9]+)\\(%(r[0-9]|r1[0-5])\\)$"
> +static regex_t sdt_op_regex1, sdt_op_regex2;
> +
> +static int sdt_init_op_regex(void)
> +{
> +	static int initialized;
> +	int ret = 0;
> +
> +	if (initialized)
> +		return 0;
> +
> +	ret = regcomp(&sdt_op_regex1, SDT_OP_REGEX1, REG_EXTENDED);
> +	if (ret)
> +		goto error;
> +	initialized = 1;
> +
> +	ret = regcomp(&sdt_op_regex2, SDT_OP_REGEX2, REG_EXTENDED);
> +	if (ret)
> +		goto free_regex1;
> +	initialized = 2;
> +
> +	return 0;
> +
> +free_regex1:
> +	regfree(&sdt_op_regex1);
> +error:
> +	pr_debug4("Regex compilation error, initialized %d\n", initialized);
> +	return ret;
> +}
> +
> +/*
> + * Parse OP and convert it into uprobe format, which is, +/-NUM(%gprREG).
> + * Possible variants of OP are:
> + *	Format		Example
> + *	-------------------------
> + *	NUM(%rREG)	48(%r1)
> + *	-NUM(%rREG)	-48(%r1)
> + *	%rREG		%r1
> + */
> +int __perf_sdt_arg_parse_op_s390(char *old_op, char **new_op)
> +{
> +	int ret, new_len;
> +	regmatch_t rm[6];
> +	unsigned long i;
> +
> +	*new_op = NULL;
> +	ret = sdt_init_op_regex();
> +	if (ret)
> +		return -EINVAL;
> +
> +	if (!regexec(&sdt_op_regex1, old_op, 3, rm, 0)) {
> +		/* Extract %rX */
> +		new_len = 2;    /* % NULL */
> +		new_len += (int)(rm[1].rm_eo - rm[1].rm_so);
> +		*new_op = zalloc(new_len);
> +		if (!*new_op)
> +			return -ENOMEM;
> +
> +		scnprintf(*new_op, new_len, "%.*s",
> +			  (int)(rm[1].rm_eo - rm[1].rm_so), old_op + rm[1].rm_so);
> +	} else if (!regexec(&sdt_op_regex2, old_op, ARRAY_SIZE(rm), rm, 0)) {
> +		/* Extract #(%rX) */
> +		new_len = 4;    /* (%)NULL */
> +		for (i = 1; i < ARRAY_SIZE(rm) && rm[i].rm_so != -1; ++i)
> +			new_len += (int)(rm[i].rm_eo - rm[i].rm_so);
> +		*new_op = zalloc(new_len);
> +		if (!*new_op)
> +			return -ENOMEM;
> +
> +		scnprintf(*new_op, new_len, "%.*s(%.*s)",
> +			  (int)(rm[1].rm_eo - rm[1].rm_so), old_op + rm[1].rm_so,
> +			  (int)(rm[2].rm_eo - rm[2].rm_so), old_op + rm[2].rm_so);
> +	} else {
> +		pr_debug4("Skipping unsupported SDT argument: %s\n", old_op);
> +		return SDT_ARG_SKIP;
> +	}
> +
> +	return SDT_ARG_VALID;
> +}
> diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
> index 5b8f34beb24e..f52b0e1f7fc7 100644
> --- a/tools/perf/util/perf_regs.c
> +++ b/tools/perf/util/perf_regs.c
> @@ -23,6 +23,9 @@ int perf_sdt_arg_parse_op(uint16_t e_machine, char *old_op, char **new_op)
>  	case EM_X86_64:
>  		ret = __perf_sdt_arg_parse_op_x86(old_op, new_op);
>  		break;
> +	case EM_S390:
> +		ret = __perf_sdt_arg_parse_op_s390(old_op, new_op);
> +		break;
>  	default:
>  		pr_debug("Unknown ELF machine %d, standard arguments parse will be skipped.\n",
>  			 e_machine);
> diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> index 7c04700bf837..573f0d1dfe04 100644
> --- a/tools/perf/util/perf_regs.h
> +++ b/tools/perf/util/perf_regs.h
> @@ -62,6 +62,7 @@ uint64_t __perf_reg_mask_s390(bool intr);
>  const char *__perf_reg_name_s390(int id);
>  uint64_t __perf_reg_ip_s390(void);
>  uint64_t __perf_reg_sp_s390(void);
> +int __perf_sdt_arg_parse_op_s390(char *old_op, char **new_op);
>  
>  int __perf_sdt_arg_parse_op_x86(char *old_op, char **new_op);
>  uint64_t __perf_reg_mask_x86(bool intr);

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

* Re: [PATCH v3] perf record: Add support for arch_sdt_arg_parse_op() on s390
  2026-03-18  1:52 ` Mi, Dapeng
@ 2026-03-18  6:27   ` Thomas Richter
  2026-03-18  8:03     ` Mi, Dapeng
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Richter @ 2026-03-18  6:27 UTC (permalink / raw)
  To: Mi, Dapeng, linux-kernel, linux-s390, linux-perf-users, acme,
	namhyung, irogers
  Cc: agordeev, gor, sumanthk, hca, japo

On 3/18/26 02:52, Mi, Dapeng wrote:
> 
> On 3/17/2026 7:06 PM, Thomas Richter wrote:
>> commit e5e66adfe45a6 ("perf regs: Remove __weak attributive arch_sdt_arg_parse_op() function")
>> removes arch_sdt_arg_parse_op() functions and s390 support is lost.
>> The following warning is printed:
> 
> Not sure if I miss something, but it looks there was also no s390 specific
> support for arch_sdt_arg_parse_op() before the commit e5e66adfe45a6 ("perf
> regs: Remove __weak attributive arch_sdt_arg_parse_op() function") and we
> would see same warning even without the commit e5e66adfe45a6, right?
> 
> 
Absolutely Correct, but in my opinion it does not matter if it was your patch or if
you just remove the __weak attribute. Your patch revealed the missing s390 support, which triggered
this patch.
If you do not like the wording, what do  you suggest?

Thanks Thomas
-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
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

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

* Re: [PATCH v3] perf record: Add support for arch_sdt_arg_parse_op() on s390
  2026-03-18  6:27   ` Thomas Richter
@ 2026-03-18  8:03     ` Mi, Dapeng
  0 siblings, 0 replies; 5+ messages in thread
From: Mi, Dapeng @ 2026-03-18  8:03 UTC (permalink / raw)
  To: Thomas Richter, linux-kernel, linux-s390, linux-perf-users, acme,
	namhyung, irogers
  Cc: agordeev, gor, sumanthk, hca, japo


On 3/18/2026 2:27 PM, Thomas Richter wrote:
> On 3/18/26 02:52, Mi, Dapeng wrote:
>> On 3/17/2026 7:06 PM, Thomas Richter wrote:
>>> commit e5e66adfe45a6 ("perf regs: Remove __weak attributive arch_sdt_arg_parse_op() function")
>>> removes arch_sdt_arg_parse_op() functions and s390 support is lost.
>>> The following warning is printed:
>> Not sure if I miss something, but it looks there was also no s390 specific
>> support for arch_sdt_arg_parse_op() before the commit e5e66adfe45a6 ("perf
>> regs: Remove __weak attributive arch_sdt_arg_parse_op() function") and we
>> would see same warning even without the commit e5e66adfe45a6, right?
>>
>>
> Absolutely Correct, but in my opinion it does not matter if it was your patch or if
> you just remove the __weak attribute. Your patch revealed the missing s390 support, which triggered
> this patch.
> If you do not like the wording, what do  you suggest?

I see. The original words lead me think the commit e5e66adfe45a6 ("perf
regs: Remove __weak attributive arch_sdt_arg_parse_op() function") drops
the s390 specific support unexpectedly. :)

So precisely speaking, we may say "the commit e5e66adfe45a6 ("perf regs:
Remove __weak attributive arch_sdt_arg_parse_op() function") introducing
perf_sdt_arg_parse_op() to support architecture-specific argument parsing,
but s390 specific argument parsing is still not supported. So this patch
adds the missing support for s390 ..."

Thanks.


>
> Thanks Thomas

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

end of thread, other threads:[~2026-03-18  8:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-17 11:06 [PATCH v3] perf record: Add support for arch_sdt_arg_parse_op() on s390 Thomas Richter
2026-03-17 15:32 ` Ian Rogers
2026-03-18  1:52 ` Mi, Dapeng
2026-03-18  6:27   ` Thomas Richter
2026-03-18  8:03     ` Mi, Dapeng

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