Re: [PATCH v5 07/14] gtrace: Add function to copy into perf AUX buffer

[Mayuresh Chitale]

Hi Zane,

On Thu, Sep 17, 2026 at 3:00 PM Zane Leung <liangzhen@xxxxxxxxxxxxxxxxxx> wrote:
>
> On 8/10/2026 11:22 PM, Mayuresh Chitale wrote:
>
> > From: Anup Patel <anup.patel@xxxxxxxxxxxxxxxx>
> >
> > The RISC-V trace ramsink will need a mechanism to copy trace data
> > into the perf AUX buffer. Add gtrace_path_copyto_auxbuf() function
> > and corresponding trace driver callback copyto_auxbuf() for this
> > purpose.
> >
> > Co-developed-by: Mayuresh Chitale <mayuresh.chitale@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Mayuresh Chitale <mayuresh.chitale@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Anup Patel <anup.patel@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/hwtracing/gtrace/gtrace-core.c | 22 ++++++++++++++++++++++
> >  include/linux/gtrace.h                 | 24 ++++++++++++++++++++++++
> >  2 files changed, 46 insertions(+)
> >
> > diff --git a/drivers/hwtracing/gtrace/gtrace-core.c b/drivers/hwtracing/gtrace/gtrace-core.c
> > index 6ff1b3087947..53a3679ba69f 100644
> > --- a/drivers/hwtracing/gtrace/gtrace-core.c
> > +++ b/drivers/hwtracing/gtrace/gtrace-core.c
> > @@ -626,6 +626,28 @@ int gtrace_path_stop(struct gtrace_path *path)
> >  }
> >  EXPORT_SYMBOL_GPL(gtrace_path_stop);
> >
> > +int gtrace_path_copyto_auxbuf(struct gtrace_path *path,
> > +                           struct gtrace_perf_auxbuf *buf,
> > +                           size_t *bytes_copied, u64 *format)
> > +{
> > +     const struct gtrace_driver *gtdrv;
> > +     struct gtrace_component *comp;
> > +     struct gtrace_path_node *node;
> > +
> > +     list_for_each_entry(node, &path->comp_list, head) {
> > +             comp = node->comp;
> > +             gtdrv = to_gtrace_driver(comp->dev.driver);
> > +             if (!gtdrv->copyto_auxbuf)
> > +                     continue;
> > +
> > +             *bytes_copied = gtdrv->copyto_auxbuf(comp, buf, format);
> > +             return 0;
> > +     }
> > +
> > +     return -EOPNOTSUPP;
> > +}
> > +EXPORT_SYMBOL_GPL(gtrace_path_copyto_auxbuf);
>
> Is it really necessary to introduce `gtrace_path_copyto_auxbuf`? The buffer copy logic should be
> exclusive to the sink component.
> A cleaner approach would be to delegate this directly to the sink instance:
> ```
> struct gtrace_component *sink = gtrace_path_sink(path);
> size = sink->copyto_auxbuf(sink, &event_data->buf, &format);
> ```
>
This looks better.
> > +
> >  struct gtrace_path *gtrace_create_path(struct gtrace_component *source,
> >                                      struct gtrace_component *sink,
> >                                      enum gtrace_component_mode mode)
> > diff --git a/include/linux/gtrace.h b/include/linux/gtrace.h
> > index 78e2ead3db8e..bc82814e2738 100644
> > --- a/include/linux/gtrace.h
> > +++ b/include/linux/gtrace.h
> > @@ -257,9 +257,30 @@ void gtrace_destroy_path(struct gtrace_path *path);
> >  int gtrace_path_start(struct gtrace_path *path);
> >  int gtrace_path_stop(struct gtrace_path *path);
> >
> > +/**
> > + * struct gtrace_perf_auxbuf - Representation of the perf AUX buffer.
> > + * @length:   Size of the AUX buffer.
> > + * @nr_pages: Number of pages of the AUX buffer.
> > + * @base:     Start address of AUX buffer.
> > + * @pos:      Position in the AUX buffer to commit traced data.
> > + */
> > +struct gtrace_perf_auxbuf {
> > +     size_t length;
> > +     int nr_pages;
> > +     void *base;
> > +     long pos;
> > +};
> > +
> > +int gtrace_path_copyto_auxbuf(struct gtrace_path *path,
> > +                           struct gtrace_perf_auxbuf *buf,
> > +                           size_t *bytes_copied, u64 *format);
> > +
> >  /**
> >   * struct gtrace_driver - Representation of a trace driver.
> >   * @id_table:      Table to match components handled by the driver.
> > + * @copyto_auxbuf: Callback to copy data into perf AUX buffer. The driver
> > + *                 reports the PMU specific trace format of the copied data
> > + *                 via @format (see PERF_AUX_FLAG_PMU_FORMAT_TYPE_MASK).
> >   * @start:         Callback to start tracing.
> >   * @stop:          Callback to stop tracing.
> >   * @probe:         Driver probe() function.
> > @@ -270,6 +291,9 @@ int gtrace_path_stop(struct gtrace_path *path);
> >   */
> >  struct gtrace_driver {
> >       const struct gtrace_component_id *id_table;
> > +     size_t                  (*copyto_auxbuf)(struct gtrace_component *comp,
> > +                                              struct gtrace_perf_auxbuf *buf,
> > +                                              u64 *format);
> >       int                     (*start)(struct gtrace_component *comp);
> >       int                     (*stop)(struct gtrace_component *comp);
> >       int                     (*probe)(struct gtrace_component *comp);