public inbox for dtrace@lists.linux.dev
 help / color / mirror / Atom feed
From: Kris Van Hees <kris.van.hees@oracle.com>
To: Eugene Loh <eugene.loh@oracle.com>
Cc: Kris Van Hees <kris.van.hees@oracle.com>,
	dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [DTrace-devel] [PATCH] sparc: remove architecture support
Date: Wed, 4 Mar 2026 15:31:26 -0500	[thread overview]
Message-ID: <aaiWnln3uGmlCu6G@oracle.com> (raw)
In-Reply-To: <483a8ddf-bc12-d7e5-f7f4-24aff9fcd874@oracle.com>

On Wed, Mar 04, 2026 at 03:07:44PM -0500, Eugene Loh wrote:
> In test/unittest/usdt/tst.enabled2.sh and
> test/unittest/aggs/tst.neglquant.d, there are SPARC comments that read funny
> in the absence of any SPARC support.

Ah, missed that.  Will deal with it.

> Should libdtrace/sparc/regs.d and include/sparc/platform.h be removed?

Yes, missed those.

> On 3/4/26 13:50, Kris Van Hees via DTrace-devel wrote:
> 
> > diff --git a/COMMANDLINE-OPTIONS b/COMMANDLINE-OPTIONS
> 
> Okay, but this file also has the line "All options are believed to work
> equally well on all supported architectures (x86 and SPARC)." How about
> s/SPARC/ARM/?

Sounds good.

> > diff --git a/GNUmakefile b/GNUmakefile
> > @@ -20,8 +20,8 @@ VERSION := $(shell ./libdtrace/mkvers -vcurrent=t libdtrace/versions.list)
> >   ARCH := $(shell uname -m)
> > -$(if $(subst sparc64,,$(subst aarch64,,$(subst x86_64,,$(ARCH)))), \
> > -    $(error "Error: DTrace for Linux only supports x86_64, ARM64 and sparc64"),)
> > +$(if $(subst aarch64,,$(subst x86_64,,$(ARCH))), \
> > +    $(error "Error: DTrace for Linux currently only supports x86_64, ARM64"),)
> 
> Okay, but how about s/x86_64, ARM64/x86_64 and ARM64/?

Sure.

> >   $(if $(subst Linux,,$(shell uname -s)), \
> >       $(error "Error: DTrace only supports Linux"),)
> > diff --git a/libdtrace/procfs.d.in b/libdtrace/procfs.d.in
> > index 23454d63..6a0e0122 100644
> > --- a/libdtrace/procfs.d.in
> > +++ b/libdtrace/procfs.d.in
> > @@ -286,8 +286,7 @@ define_for_kernel([[on_cpu]], [[(m4_kver(5,16,0), m4_dnl
> >     [[((struct thread_info *)T->stack)->cpu]]), m4_dnl
> >   (m4_kver(4,9,0), m4_dnl
> >     [[if_arch([[x86]],[[T->cpu]])]] m4_dnl
> > -  [[if_arch([[arm64]],[[T->cpu]])]] m4_dnl
> > -  [[if_arch([[sparc]],[[((struct thread_info *)T->stack)->cpu]])]])]], m4_dnl
> > +  [[if_arch([[arm64]],[[T->cpu]])]]l)]], m4_dnl
> >   [[((struct thread_info *)T->stack)->cpu]])m4_dnl
> >   	pr_onpro = on_cpu;
> >   };
> 
> I can't read M4, but maybe that diff is suspicious?  This is what gpt 5.2
> says:
> 
> ### USER
> Does the following diff in an m4 file make sense?  Are brackets still
> balanced?
> @@ -286,8 +286,7 @@ define_for_kernel([[on_cpu]], [[(m4_kver(5,16,0), m4_dnl
>    [[((struct thread_info *)T->stack)->cpu]]), m4_dnl
> (m4_kver(4,9,0), m4_dnl
>    [[if_arch([[x86]],[[T->cpu]])]] m4_dnl
> -  [[if_arch([[arm64]],[[T->cpu]])]] m4_dnl
> -  [[if_arch([[sparc]],[[((struct thread_info *)T->stack)->cpu]])]])]],
> m4_dnl
> +  [[if_arch([[arm64]],[[T->cpu]])]]l)]], m4_dnl
> [[((struct thread_info *)T->stack)->cpu]])m4_dnl
>         pr_onpro = on_cpu;
> };
> 
> ### ASSISTANT
> No - as shown, that diff does **not** make sense, and the
> brackets/parentheses are very likely **not balanced** anymore (and there's
> also an obvious stray character).
> 
> ### What looks wrong
> 
> 1. **Stray `l` and unmatched delimiters**

The stray 'l' is indeed a bug.  Fixed.

The rest the AI bot got wrong.

>    ```
>    [[if_arch([[arm64]],[[T->cpu]])]]l)]], m4_dnl
>                               ^^^
>    ```
>    That `l)` sequence is almost certainly accidental. It also strongly
> suggests you now have an extra `)` and/or `]` compared to what was intended.
> 
> 2. **You removed the `sparc` branch and spliced the "default" expression
> oddly**
>    Originally you had:
>    ```
>    ... [[if_arch([[arm64]],[[T->cpu]])]] m4_dnl
>        [[if_arch([[sparc]],[[((struct thread_info *)T->stack)->cpu]])]])]],
> m4_dnl
>    ```
>    That last line clearly closes a number of nested `[[ ... ]]` / `(...)`
> constructs.
> 
>    In the new version, you have:
>    ```
>    [[if_arch([[arm64]],[[T->cpu]])]]l)]], m4_dnl
>    [[((struct thread_info *)T->stack)->cpu]])m4_dnl
>    ```
>    The "default" `[[((struct thread_info *)T->stack)->cpu]]` expression now
> appears outside of the previous nesting, and the closers that used to be on
> the `sparc` line are now partially (and incorrectly) attached to the `arm64`
> line.
> 
> 3. **`m4_dnl` placement**
>    `m4_dnl` comments out the rest of the line. Moving delimiters to a new
> line while also changing where `m4_dnl` lands can easily break balanced
> delimiters, because closers might end up being "commented away" or no longer
> part of the intended argument.
> 
> ### Practical conclusion
> - **Brackets are almost certainly unbalanced** after this edit.
> - The diff as shown looks like a broken manual edit (typo + moved closers).
> 
> ### What to do next
> - If your intent was only to remove the `sparc` case, the safe approach is:
> remove just that `if_arch([[sparc]]...)` fragment but keep the closing
> `]])]],` structure intact (i.e., don't move closers onto other lines).
> - Run `m4` on the file (or whatever build step consumes it) and expect it to
> fail with a quoting/argument/parenthesis error if this is truly the current
> state.
> 
> If you paste ~10-20 lines around this macro (before/after), I can tell you
> exactly where the missing/extra `[[` / `]]` / `(` / `)` are and suggest a
> corrected minimal diff.

      parent reply	other threads:[~2026-03-04 20:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-04 18:50 [PATCH] sparc: remove architecture support Kris Van Hees
2026-03-04 20:07 ` [DTrace-devel] " Eugene Loh
2026-03-04 20:25   ` Nick Alcock
2026-03-04 20:31   ` Kris Van Hees [this message]

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=aaiWnln3uGmlCu6G@oracle.com \
    --to=kris.van.hees@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    --cc=eugene.loh@oracle.com \
    /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