Linux Documentation
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com>
Cc: Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	skhan@linuxfoundation.org, rdunlap@infradead.org,
	mchehab@kernel.org, paulmck@kernel.org, linusw@kernel.org,
	Julia.Lawall@inria.fr, gary@garyguo.net,
	rmk+kernel@armlinux.org.uk, hemanth.selam@gmail.com
Subject: Re: [PATCH] docs: kdoc: handle TRACE_EVENT_FN() family in tracepoint parser
Date: Fri, 16 Oct 2026 23:11:48 +0200	[thread overview]
Message-ID: <20261016231148.7b5a88ef@foz.lan> (raw)
In-Reply-To: <20261016204335.46241-1-emmaonana18@gmail.com>

On Fri, 16 Oct 2026 21:43:35 +0100
Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com> wrote:

> kernel-doc fails to parse tracepoint declarations that use
> TRACE_EVENT_FN(), e.g. gpu_power_status:
> 
>   drivers/gpu/drm/panthor/panthor_trace.h:28 ... Unrecognized
>   tracepoint format ...
>   ... expecting prototype for gpu_power_status(). Prototype was for
>   TRACE_EVENT_FN() instead
> 
> tracepoint_munge() only matches TRACE_EVENT(), so the macro name is
> kept as the prototype and the TP_PROTO() arguments leak into the
> parameter list as garbage.
> 
> Match TRACE_EVENT_FN(), TRACE_EVENT_FN_COND(),
> TRACE_EVENT_CONDITION() and TRACE_EVENT_SYSCALL() the same way as
> TRACE_EVENT()
> 
> Add a regression test for the gpu_power_status declaration.
> 
> Signed-off-by: Omokefe Emmanuel Onanaroghene <emmaonana18@gmail.com>
> ---
> Move tracepoint handling to xforms_lists, together with SYSCALL_DEFINE
> as requested by Mauro.
> 
> Question for Mauro: is adding syscall_munge() and
> tracepoint_munge() in apply() what you meant, or
> did you want a pure CMatch? 

I meant a pure CMatch. One of the hardest things to maintain at
kernel-doc are those complex regex logic spread all over it. The
goal for CMatch and the C tokenizer is to avoid adding more - and 
instead - to reduce the existing ones over time.

In the specific case of commas, the tokenizer, ideally double commas
should be generating a singe STRING token and single commas a single
CHAR.

> A pure CMatch looks
> like it would need a tokenizer fix for nested
> commas, so I'd prefer to keep it this way unless
> you want the pure CMatch rework.

If the nested commas logic there is broken, indeed it needs a fix, on a 
separate patch.

> 
>  tools/lib/python/kdoc/kdoc_parser.py  | 110 ++++++--------------------
>  tools/lib/python/kdoc/xforms_lists.py | 109 +++++++++++++++++++++++++
>  tools/unittests/test_kdoc_parser.py   |  28 +++++++
>  3 files changed, 159 insertions(+), 88 deletions(-)
> 
> diff --git a/tools/lib/python/kdoc/kdoc_parser.py b/tools/lib/python/kdoc/kdoc_parser.py
> index 80d4245acd17..a274e06b9f23 100644
> --- a/tools/lib/python/kdoc/kdoc_parser.py
> +++ b/tools/lib/python/kdoc/kdoc_parser.py
> @@ -1059,8 +1059,27 @@ class KernelDoc:
>              #
>              # Apply the initial transformations.
>              #
> +            # This also handles SYSCALL_DEFINE*() and TRACE_EVENT*()/
> +            # DEFINE_EVENT*() via CTransforms (see xforms_lists.py).
> +            #
>              prototype = self.xforms.apply("func", prototype)
>  
> +            #
> +            # Tracepoints are documented with their short name, e.g.
> +            # gpu_power_status for trace_gpu_power_status().
> +            # Accept the short name and store the full trace_ name.
> +            #
> +            r = KernRe(r'\btrace_(\w+)')
> +            if r.search(prototype):
> +                if self.entry.identifier == r.group(1):
> +                    self.entry.identifier = f"trace_{self.entry.identifier}"
> +
> +
> +            r = KernRe(r'TRACE_EVENT|DEFINE_EVENT|DEFINE_SINGLE_EVENT')
> +            if r.search(prototype):
> +                self.emit_msg(ln,
> +                              f"Unrecognized tracepoint format:\n{prototype}\n")
> +
>          # Yes, this truly is vile.  We are looking for:
>          # 1. Return type (may be nothing if we're looking at a macro)
>          # 2. Function name
> @@ -1493,82 +1512,6 @@ class KernelDoc:
>              self.entry.add_text(doc_content.group(1))
>          # else ... ??
>  
> -    def syscall_munge(self, ln, proto):         # pylint: disable=W0613
> -        """
> -        Handle syscall definitions.
> -        """
> -
> -        is_void = False
> -
> -        # Strip newlines/CR's
> -        proto = re.sub(r'[\r\n]+', ' ', proto)
> -
> -        # Check if it's a SYSCALL_DEFINE0
> -        if 'SYSCALL_DEFINE0' in proto:
> -            is_void = True
> -
> -        # Replace SYSCALL_DEFINE with correct return type & function name
> -        proto = KernRe(r'SYSCALL_DEFINE.*\(').sub('long sys_', proto)
> -
> -        r = KernRe(r'long\s+(sys_.*?),')
> -        if r.search(proto):
> -            proto = KernRe(',').sub('(', proto, count=1)
> -        elif is_void:
> -            proto = KernRe(r'\)').sub('(void)', proto, count=1)
> -
> -        # Now delete all of the odd-numbered commas in the proto
> -        # so that argument types & names don't have a comma between them
> -        count = 0
> -        length = len(proto)
> -
> -        if is_void:
> -            length = 0  # skip the loop if is_void
> -
> -        for ix in range(length):
> -            if proto[ix] == ',':
> -                count += 1
> -                if count % 2 == 1:
> -                    proto = proto[:ix] + ' ' + proto[ix + 1:]
> -
> -        return proto
> -
> -    def tracepoint_munge(self, ln, proto):
> -        """
> -        Handle tracepoint definitions.
> -        """
> -
> -        tracepointname = None
> -        tracepointargs = None
> -
> -        # Match tracepoint name based on different patterns
> -        r = KernRe(r'TRACE_EVENT\((.*?),')
> -        if r.search(proto):
> -            tracepointname = r.group(1)
> -
> -        r = KernRe(r'DEFINE_SINGLE_EVENT\((.*?),')
> -        if r.search(proto):
> -            tracepointname = r.group(1)
> -
> -        r = KernRe(r'DEFINE_EVENT\((.*?),(.*?),')
> -        if r.search(proto):
> -            tracepointname = r.group(2)
> -
> -        if tracepointname:
> -            tracepointname = tracepointname.lstrip()
> -
> -        r = KernRe(r'TP_PROTO\((.*?)\)')
> -        if r.search(proto):
> -            tracepointargs = r.group(1)
> -
> -        if not tracepointname or not tracepointargs:
> -            self.emit_msg(ln,
> -                          f"Unrecognized tracepoint format:\n{proto}\n")
> -        else:
> -            proto = f"static inline void trace_{tracepointname}({tracepointargs})"
> -            self.entry.identifier = f"trace_{self.entry.identifier}"
> -
> -        return proto
> -
>      def process_proto_function(self, ln, line, source):
>          """Ancillary routine to process a function prototype."""
>  
> @@ -1598,18 +1541,9 @@ class KernelDoc:
>              r = KernRe(r'^(\S+\s+)\(\s*\*(\S+)\)')
>              self.entry.prototype = r.sub(r'\1\2', self.entry.prototype)
>              #
> -            # Handle special declaration syntaxes
> -            #
> -            if 'SYSCALL_DEFINE' in self.entry.prototype:
> -                self.entry.prototype = self.syscall_munge(ln,
> -                                                          self.entry.prototype)
> -            else:
> -                r = KernRe(r'TRACE_EVENT|DEFINE_EVENT|DEFINE_SINGLE_EVENT')
> -                if r.search(self.entry.prototype):
> -                    self.entry.prototype = self.tracepoint_munge(ln,
> -                                                                 self.entry.prototype)
> -            #
> -            # ... and we're done
> +            # ... and we're done. SYSCALL_DEFINE*() and TRACE_EVENT*()/
> +            # DEFINE_EVENT*() are handled via xforms_lists.CTransforms
> +            # inside dump_function().
>              #
>              self.dump_function(ln, self.entry.prototype, source)
>              self.reset_state(ln)
> diff --git a/tools/lib/python/kdoc/xforms_lists.py b/tools/lib/python/kdoc/xforms_lists.py
> index 15f6f21c7a9e..aa82dc927ef1 100644
> --- a/tools/lib/python/kdoc/xforms_lists.py
> +++ b/tools/lib/python/kdoc/xforms_lists.py
> @@ -17,6 +17,98 @@ class CTransforms:
>      into something we can parse and generate kdoc for.
>      """
>  
> +    @staticmethod
> +    def syscall_munge(proto):
> +        """
> +        Handle syscall definitions.
> +
> +        Turns SYSCALL_DEFINE*() into a plain C prototype, e.g.::
> +
> +            SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
> +            -> long sys_setpriority(int which, int who, int niceval)
> +        """
> +
> +        is_void = False
> +
> +        # Strip newlines/CR's
> +        proto = re.sub(r'[\r\n]+', ' ', proto)
> +
> +        # Check if it's a SYSCALL_DEFINE0
> +        if 'SYSCALL_DEFINE0' in proto:
> +            is_void = True
> +
> +        # Replace SYSCALL_DEFINE with correct return type & function name
> +        proto = KernRe(r'SYSCALL_DEFINE.*\(').sub('long sys_', proto)
> +
> +        r = KernRe(r'long\s+(sys_.*?),')
> +        if r.search(proto):
> +            proto = KernRe(',').sub('(', proto, count=1)
> +        elif is_void:
> +            proto = KernRe(r'\)').sub('(void)', proto, count=1)
> +
> +        # Now delete all of the odd-numbered commas in the proto
> +        # so that argument types & names don't have a comma between them
> +        count = 0
> +        length = len(proto)
> +
> +        if is_void:
> +            length = 0  # skip the loop if is_void
> +
> +        for ix in range(length):
> +            if proto[ix] == ',':
> +                count += 1
> +                if count % 2 == 1:
> +                    proto = proto[:ix] + ' ' + proto[ix + 1:]
> +
> +        return proto
> +
> +    @staticmethod
> +    def tracepoint_munge(proto):
> +        """
> +        Handle tracepoint definitions.
> +
> +        Turns TRACE_EVENT*()/DEFINE_EVENT*() into a plain C prototype,
> +        e.g.::
> +
> +            TRACE_EVENT_FN(gpu_power_status, TP_PROTO(...), ...)
> +            -> static inline void trace_gpu_power_status(...)
> +
> +        Returns the original string if the tracepoint format is not
> +        recognized, letting the caller emit a warning.
> +        """
> +
> +        tracepointname = None
> +        tracepointargs = None
> +
> +        # Match tracepoint name based on different patterns.
> +        # NOTE: DEFINE_EVENT*() takes a template name first, so the
> +        # event name is the second argument.
> +        for pattern, group in [
> +            (r'TRACE_EVENT\((.*?),', 1),
> +            (r'TRACE_EVENT_FN\((.*?),', 1),
> +            (r'TRACE_EVENT_FN_COND\((.*?),', 1),
> +            (r'TRACE_EVENT_CONDITION\((.*?),', 1),
> +            (r'TRACE_EVENT_SYSCALL\((.*?),', 1),
> +            (r'DEFINE_SINGLE_EVENT\((.*?),', 1),
> +            (r'DEFINE_EVENT\((.*?),(.*?),', 2),
> +        ]:
> +            r = KernRe(pattern)
> +            if r.search(proto):
> +                tracepointname = r.group(group)
> +
> +        if tracepointname:
> +            tracepointname = tracepointname.lstrip()
> +
> +        r = KernRe(r'TP_PROTO\((.*?)\)')
> +        if r.search(proto):
> +            tracepointargs = r.group(1)
> +
> +        if not tracepointname or not tracepointargs:
> +            return proto
> +
> +        return f"static inline void trace_{tracepointname}({tracepointargs})"
> +
> +
>      #
>      # NOTE:
>      #      Due to performance reasons, place CMatch rules before KernRe,
> @@ -147,6 +239,23 @@ class CTransforms:
>          if xforms_type not in self.xforms:
>              return source
>  
> +        #
> +        # Handle special declaration syntaxes first, so that the
> +        # declarative transforms below see plain C prototypes.
> +        #
> +        if xforms_type == "func":
> +            source_str = str(source)
> +            if 'SYSCALL_DEFINE' in source_str:
> +                source_str = self.syscall_munge(source_str)
> +                source = source_str
> +            else:
> +                r = KernRe(r'TRACE_EVENT|DEFINE_EVENT|DEFINE_SINGLE_EVENT')
> +                if r.search(source_str):
> +                    munged = self.tracepoint_munge(source_str)
> +                    # tracepoint_munge() returns the input unchanged
> +                    # when the format is not recognized.
> +                    source = munged
> +
>          if isinstance(source, str):
>              source = CTokenizer(source)
>  
> diff --git a/tools/unittests/test_kdoc_parser.py b/tools/unittests/test_kdoc_parser.py
> index 4baa78ca02b3..04dafae412e4 100755
> --- a/tools/unittests/test_kdoc_parser.py
> +++ b/tools/unittests/test_kdoc_parser.py
> @@ -444,6 +444,34 @@ class TestContextLockStruct(GenerateKdocItem):
>          self.assertEqual(entries[0].name, "debugfs_cancellation")
>          self.assertEqual(entries[0].warnings, [])
>  
> +class TestTraceEventFn(GenerateKdocItem):
> +    def test_trace_event_fn_declaration(self):
> +        source = """
> +            /**
> +             * gpu_power_status - called when hardware turns on or off
> +             * @dev: pointer to device
> +             * @shader_bitmap: bitmap
> +             */
> +            TRACE_EVENT_FN(gpu_power_status,
> +                TP_PROTO(const struct device *dev, u64 shader_bitmap),
> +                TP_ARGS(dev, shader_bitmap),
> +                TP_STRUCT__entry(__field(u64, shader_bitmap)),
> +                TP_fast_assign(__entry->shader_bitmap = shader_bitmap;),
> +                TP_printk("%s", __get_str(dev_name)),
> +                my_register, my_unregister
> +            );
> +        """
> +
> +        kernel_doc = KernelDoc(self.config, "test.c", self.xforms)
> +        patcher = patch('builtins.open', new_callable=mock_open,
> +                        read_data=dedent(source))
> +        with patcher:
> +            _, entries = kernel_doc.parse_kdoc()
> +
> +        self.assertEqual(len(entries), 1)
> +        self.assertEqual(entries[0].name, "trace_gpu_power_status")
> +        self.assertEqual(entries[0].warnings, [])
> +
>  #
>  # Class and logic to create dynamic tests from YAML
>  #



Thanks,
Mauro

  reply	other threads:[~2026-10-16 21:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-02  4:15 [PATCH] docs: kdoc: strip DOMAIN_NS() wrapper in function prototypes Omokefe Emmanuel Onanaroghene
2026-10-02  4:28 ` Randy Dunlap
2026-10-14 17:22 ` Jonathan Corbet
2026-10-14 22:47   ` Omokefe Emmanuel Onanaroghene
2026-10-15  0:53     ` Omokefe Emmanuel Onanaroghene
2026-10-15 13:00       ` Jonathan Corbet
2026-10-15 16:44         ` [PATCH v3 0/2] docs: kdoc: fix TRACE_EVENT_FN and DOMAIN_NS parsing Omokefe Emmanuel Onanaroghene
2026-10-15 16:44           ` [PATCH v3 1/2] docs: kdoc: handle TRACE_EVENT_FN() family in tracepoint parser Omokefe Emmanuel Onanaroghene
2026-10-15 17:06             ` Mauro Carvalho Chehab
2026-10-16 20:43               ` [PATCH] " Omokefe Emmanuel Onanaroghene
2026-10-16 21:11                 ` Mauro Carvalho Chehab [this message]
2026-10-15 16:44           ` [PATCH v3 2/2] docs: kdoc: strip DOMAIN_NS() wrapper in function prototypes Omokefe Emmanuel Onanaroghene
  -- strict thread matches above, loose matches on Subject: below --
2026-10-02  2:34 [PATCH] docs: kdoc: handle TRACE_EVENT_FN() family in tracepoint parser Omokefe Emmanuel Onanaroghene
2026-10-02  4:21 ` Randy Dunlap

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=20261016231148.7b5a88ef@foz.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=Julia.Lawall@inria.fr \
    --cc=corbet@lwn.net \
    --cc=emmaonana18@gmail.com \
    --cc=gary@garyguo.net \
    --cc=hemanth.selam@gmail.com \
    --cc=linusw@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=skhan@linuxfoundation.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