| Message ID | 1445192687-24112-20-git-send-email-mathieu.poirier@linaro.org |
|---|---|
| State | New |
| Headers | show |
Mathieu Poirier <mathieu.poirier@linaro.org> writes: > Adding an ETB10 specific auxiliary area setup operation to be > used by the perf framework when events are initialised. > > Part of this operation involves modeling the mmap'ed area based > on the specific ways a sink buffer gathers information. It really doesn't seem to be ETB10 specific at all. When you add more sinks, you'll probably end up copying this code every time. Furthermore, > +static void *etb_setup_aux(struct coresight_device *csdev, int cpu, > + void **pages, int nr_pages, bool overwrite) > +{ > + int node, pg; > + struct cs_buffers *buf; > + > + if (cpu == -1) > + cpu = smp_processor_id(); > + node = cpu_to_node(cpu); > + > + buf = kzalloc_node(offsetof(struct cs_buffers, addr[nr_pages]), > + GFP_KERNEL, node); > + if (!buf) > + return NULL; > + > + buf->snapshot = overwrite; > + buf->nr_pages = nr_pages; > + > + /* Record information about buffers */ > + for (pg = 0; pg < buf->nr_pages; pg++) > + buf->addr[pg] = pages[pg]; > + > + return buf; > +} > + this one is so generic that I'm tempted to move this into perf's ring_buffer code, because by the looks of it we'll need it pretty much in every setup_aux(). Regards, -- Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Mathieu Poirier <mathieu.poirier@linaro.org> writes: > /** > + * struct cs_buffer - keep track of a recording session' specifics > + * @cur: index of the current buffer > + * @nr_pages: max number of pages granted to us > + * @nr_bufs: number of clustered pages > + * @offset: offset within the current buffer > + * @size: how much space we have for this run > + * @data_size: how much we collected in this run > + * @head: head of the ring buffer > + * @lost: other than zero if we had a HW buffer wrap around > + * @snapshot: is this run in snapshot mode > + * @addr: virtual address this buffer starts at > + */ > +struct cs_buffers { > + unsigned int cur; > + unsigned int nr_pages; > + unsigned int nr_bufs; This one is not really used. > + unsigned long offset; > + unsigned long size; And this one seems to be only set in one place. > + local_t data_size; > + local_t head; And so is this one. > + local_t lost; > + bool snapshot; > + void *addr[0]; And this one seems to be a copy of what perf's ring buffer gives us. > +static void *etb_setup_aux(struct coresight_device *csdev, int cpu, > + void **pages, int nr_pages, bool overwrite) > +{ > + int node, pg; > + struct cs_buffers *buf; > + > + if (cpu == -1) > + cpu = smp_processor_id(); > + node = cpu_to_node(cpu); > + > + buf = kzalloc_node(offsetof(struct cs_buffers, addr[nr_pages]), > + GFP_KERNEL, node); > + if (!buf) > + return NULL; > + > + buf->snapshot = overwrite; > + buf->nr_pages = nr_pages; > + > + /* Record information about buffers */ > + for (pg = 0; pg < buf->nr_pages; pg++) > + buf->addr[pg] = pages[pg]; Yes, buf::addr is a copy of @pages. You could save some space by just saving @pages, it's going to be around until pmu::free_aux(). Regards, -- Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 19 October 2015 at 07:44, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Mathieu Poirier <mathieu.poirier@linaro.org> writes: > >> Adding an ETB10 specific auxiliary area setup operation to be >> used by the perf framework when events are initialised. >> >> Part of this operation involves modeling the mmap'ed area based >> on the specific ways a sink buffer gathers information. > > It really doesn't seem to be ETB10 specific at all. When you add more > sinks, you'll probably end up copying this code every time. That will depend on how that specific sinks work, but indeed, it is pretty generic. > > Furthermore, > >> +static void *etb_setup_aux(struct coresight_device *csdev, int cpu, >> + void **pages, int nr_pages, bool overwrite) >> +{ >> + int node, pg; >> + struct cs_buffers *buf; >> + >> + if (cpu == -1) >> + cpu = smp_processor_id(); >> + node = cpu_to_node(cpu); >> + >> + buf = kzalloc_node(offsetof(struct cs_buffers, addr[nr_pages]), >> + GFP_KERNEL, node); >> + if (!buf) >> + return NULL; >> + >> + buf->snapshot = overwrite; >> + buf->nr_pages = nr_pages; >> + >> + /* Record information about buffers */ >> + for (pg = 0; pg < buf->nr_pages; pg++) >> + buf->addr[pg] = pages[pg]; >> + >> + return buf; >> +} >> + > > this one is so generic that I'm tempted to move this into perf's > ring_buffer code, because by the looks of it we'll need it pretty much > in every setup_aux(). It is tempting but I suggest we wait to see what kind of trend we get before moving ahead with this. There is always opportunity for further consolidation should the need arise. > > Regards, > -- > Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index ecdbe0dd4d08..7f34e7af465e 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -27,6 +27,9 @@ #include <linux/coresight.h> #include <linux/amba/bus.h> #include <linux/clk.h> +#include <linux/mm.h> + +#include <asm/local.h> #include "coresight-priv.h" @@ -64,6 +67,32 @@ #define ETB_FRAME_SIZE_WORDS 4 /** + * struct cs_buffer - keep track of a recording session' specifics + * @cur: index of the current buffer + * @nr_pages: max number of pages granted to us + * @nr_bufs: number of clustered pages + * @offset: offset within the current buffer + * @size: how much space we have for this run + * @data_size: how much we collected in this run + * @head: head of the ring buffer + * @lost: other than zero if we had a HW buffer wrap around + * @snapshot: is this run in snapshot mode + * @addr: virtual address this buffer starts at + */ +struct cs_buffers { + unsigned int cur; + unsigned int nr_pages; + unsigned int nr_bufs; + unsigned long offset; + unsigned long size; + local_t data_size; + local_t head; + local_t lost; + bool snapshot; + void *addr[0]; +}; + +/** * struct etb_drvdata - specifics associated to an ETB component * @base: memory mapped base address for this component. * @dev: the device entity associated to this component. @@ -252,9 +281,35 @@ static void etb_disable(struct coresight_device *csdev) dev_info(drvdata->dev, "ETB disabled\n"); } +static void *etb_setup_aux(struct coresight_device *csdev, int cpu, + void **pages, int nr_pages, bool overwrite) +{ + int node, pg; + struct cs_buffers *buf; + + if (cpu == -1) + cpu = smp_processor_id(); + node = cpu_to_node(cpu); + + buf = kzalloc_node(offsetof(struct cs_buffers, addr[nr_pages]), + GFP_KERNEL, node); + if (!buf) + return NULL; + + buf->snapshot = overwrite; + buf->nr_pages = nr_pages; + + /* Record information about buffers */ + for (pg = 0; pg < buf->nr_pages; pg++) + buf->addr[pg] = pages[pg]; + + return buf; +} + static const struct coresight_ops_sink etb_sink_ops = { .enable = etb_enable, .disable = etb_disable, + .setup_aux = etb_setup_aux, }; static const struct coresight_ops etb_cs_ops = { diff --git a/include/linux/coresight.h b/include/linux/coresight.h index dd530a6d3e21..b1c25eba83b4 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -187,10 +187,13 @@ struct coresight_device { * Operations available for sinks * @enable: enables the sink. * @disable: disables the sink. + * @setup_aux: initialises perf's ring buffer for trace collection. */ struct coresight_ops_sink { int (*enable)(struct coresight_device *csdev); void (*disable)(struct coresight_device *csdev); + void *(*setup_aux)(struct coresight_device *csdev, int cpu, + void **pages, int nr_pages, bool overwrite); }; /**
Adding an ETB10 specific auxiliary area setup operation to be used by the perf framework when events are initialised. Part of this operation involves modeling the mmap'ed area based on the specific ways a sink buffer gathers information. Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> --- drivers/hwtracing/coresight/coresight-etb10.c | 55 +++++++++++++++++++++++++++ include/linux/coresight.h | 3 ++ 2 files changed, 58 insertions(+)