diff mbox series

[4/6] perf/aux: Make perf_event accessible to setup_aux()

Message ID 1530570810-28929-5-git-send-email-mathieu.poirier@linaro.org
State Superseded
Headers show
Series perf: Add ioctl for PMU driver configuration | expand

Commit Message

Mathieu Poirier July 2, 2018, 10:33 p.m. UTC 
It can be advantagous to have access to all the information conveyed by
a perf_event when setting up the AUX buffer, as it is the case when
dealing with PMU specific driver configuration communicated to the kernel
using an ioctl() call.

As such simply replace the cpu information by the complete perf_event
structure and change all affected customers.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

---
 arch/s390/kernel/perf_cpum_sf.c                  | 4 ++--
 arch/x86/events/intel/bts.c                      | 4 +++-
 arch/x86/events/intel/pt.c                       | 5 +++--
 drivers/hwtracing/coresight/coresight-etm-perf.c | 6 +++---
 drivers/perf/arm_spe_pmu.c                       | 6 +++---
 include/linux/perf_event.h                       | 2 +-
 kernel/events/ring_buffer.c                      | 2 +-
 7 files changed, 16 insertions(+), 13 deletions(-)

-- 
2.7.4

Comments

kernel test robot July 3, 2018, 2:05 a.m. UTC | #1 
Hi Mathieu,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.18-rc3 next-20180702]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mathieu-Poirier/perf-Add-ioctl-for-PMU-driver-configuration/20180703-064327
config: s390-defconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=s390 

All error/warnings (new ones prefixed by >>):

>> arch/s390/kernel/perf_cpum_sf.c:1606:1: error: expected identifier or '(' before '{' token

    {
    ^
>> arch/s390/kernel/perf_cpum_sf.c:1604:14: warning: 'aux_buffer_setup' used but never defined

    static void *aux_buffer_setup(struct perf_event *event, void **pages,
                 ^~~~~~~~~~~~~~~~

vim +1606 arch/s390/kernel/perf_cpum_sf.c

ca5955cd Pu Hou          2016-11-11  1589  
ca5955cd Pu Hou          2016-11-11  1590  /*
ca5955cd Pu Hou          2016-11-11  1591   * aux_buffer_setup() - Setup AUX buffer for diagnostic mode sampling
ca5955cd Pu Hou          2016-11-11  1592   * @cpu:	On which to allocate, -1 means current
ca5955cd Pu Hou          2016-11-11  1593   * @pages:	Array of pointers to buffer pages passed from perf core
ca5955cd Pu Hou          2016-11-11  1594   * @nr_pages:	Total pages
ca5955cd Pu Hou          2016-11-11  1595   * @snapshot:	Flag for snapshot mode
ca5955cd Pu Hou          2016-11-11  1596   *
ca5955cd Pu Hou          2016-11-11  1597   * This is the callback when setup an event using AUX buffer. Perf tool can
ca5955cd Pu Hou          2016-11-11  1598   * trigger this by an additional mmap() call on the event. Unlike the buffer
ca5955cd Pu Hou          2016-11-11  1599   * for basic samples, AUX buffer belongs to the event. It is scheduled with
ca5955cd Pu Hou          2016-11-11  1600   * the task among online cpus when it is a per-thread event.
ca5955cd Pu Hou          2016-11-11  1601   *
ca5955cd Pu Hou          2016-11-11  1602   * Return the private AUX buffer structure if success or NULL if fails.
ca5955cd Pu Hou          2016-11-11  1603   */
ceb39bf0 Mathieu Poirier 2018-07-02 @1604  static void *aux_buffer_setup(struct perf_event *event, void **pages,
ceb39bf0 Mathieu Poirier 2018-07-02  1605  			      int nr_pages, bool snapshot);
ca5955cd Pu Hou          2016-11-11 @1606  {
ca5955cd Pu Hou          2016-11-11  1607  	struct sf_buffer *sfb;
ca5955cd Pu Hou          2016-11-11  1608  	struct aux_buffer *aux;
ca5955cd Pu Hou          2016-11-11  1609  	unsigned long *new, *tail;
ca5955cd Pu Hou          2016-11-11  1610  	int i, n_sdbt;
ca5955cd Pu Hou          2016-11-11  1611  
ca5955cd Pu Hou          2016-11-11  1612  	if (!nr_pages || !pages)
ca5955cd Pu Hou          2016-11-11  1613  		return NULL;
ca5955cd Pu Hou          2016-11-11  1614  
ca5955cd Pu Hou          2016-11-11  1615  	if (nr_pages > CPUM_SF_MAX_SDB * CPUM_SF_SDB_DIAG_FACTOR) {
ca5955cd Pu Hou          2016-11-11  1616  		pr_err("AUX buffer size (%i pages) is larger than the "
ca5955cd Pu Hou          2016-11-11  1617  		       "maximum sampling buffer limit\n",
ca5955cd Pu Hou          2016-11-11  1618  		       nr_pages);
ca5955cd Pu Hou          2016-11-11  1619  		return NULL;
ca5955cd Pu Hou          2016-11-11  1620  	} else if (nr_pages < CPUM_SF_MIN_SDB * CPUM_SF_SDB_DIAG_FACTOR) {
ca5955cd Pu Hou          2016-11-11  1621  		pr_err("AUX buffer size (%i pages) is less than the "
ca5955cd Pu Hou          2016-11-11  1622  		       "minimum sampling buffer limit\n",
ca5955cd Pu Hou          2016-11-11  1623  		       nr_pages);
ca5955cd Pu Hou          2016-11-11  1624  		return NULL;
ca5955cd Pu Hou          2016-11-11  1625  	}
ca5955cd Pu Hou          2016-11-11  1626  
ca5955cd Pu Hou          2016-11-11  1627  	/* Allocate aux_buffer struct for the event */
ca5955cd Pu Hou          2016-11-11  1628  	aux = kmalloc(sizeof(struct aux_buffer), GFP_KERNEL);
ca5955cd Pu Hou          2016-11-11  1629  	if (!aux)
ca5955cd Pu Hou          2016-11-11  1630  		goto no_aux;
ca5955cd Pu Hou          2016-11-11  1631  	sfb = &aux->sfb;
ca5955cd Pu Hou          2016-11-11  1632  
ca5955cd Pu Hou          2016-11-11  1633  	/* Allocate sdbt_index for fast reference */
ca5955cd Pu Hou          2016-11-11  1634  	n_sdbt = (nr_pages + CPUM_SF_SDB_PER_TABLE - 1) / CPUM_SF_SDB_PER_TABLE;
ca5955cd Pu Hou          2016-11-11  1635  	aux->sdbt_index = kmalloc_array(n_sdbt, sizeof(void *), GFP_KERNEL);
ca5955cd Pu Hou          2016-11-11  1636  	if (!aux->sdbt_index)
ca5955cd Pu Hou          2016-11-11  1637  		goto no_sdbt_index;
ca5955cd Pu Hou          2016-11-11  1638  
ca5955cd Pu Hou          2016-11-11  1639  	/* Allocate sdb_index for fast reference */
ca5955cd Pu Hou          2016-11-11  1640  	aux->sdb_index = kmalloc_array(nr_pages, sizeof(void *), GFP_KERNEL);
ca5955cd Pu Hou          2016-11-11  1641  	if (!aux->sdb_index)
ca5955cd Pu Hou          2016-11-11  1642  		goto no_sdb_index;
ca5955cd Pu Hou          2016-11-11  1643  
ca5955cd Pu Hou          2016-11-11  1644  	/* Allocate the first SDBT */
ca5955cd Pu Hou          2016-11-11  1645  	sfb->num_sdbt = 0;
ca5955cd Pu Hou          2016-11-11  1646  	sfb->sdbt = (unsigned long *) get_zeroed_page(GFP_KERNEL);
ca5955cd Pu Hou          2016-11-11  1647  	if (!sfb->sdbt)
ca5955cd Pu Hou          2016-11-11  1648  		goto no_sdbt;
ca5955cd Pu Hou          2016-11-11  1649  	aux->sdbt_index[sfb->num_sdbt++] = (unsigned long)sfb->sdbt;
ca5955cd Pu Hou          2016-11-11  1650  	tail = sfb->tail = sfb->sdbt;
ca5955cd Pu Hou          2016-11-11  1651  
ca5955cd Pu Hou          2016-11-11  1652  	/*
ca5955cd Pu Hou          2016-11-11  1653  	 * Link the provided pages of AUX buffer to SDBT.
ca5955cd Pu Hou          2016-11-11  1654  	 * Allocate SDBT if needed.
ca5955cd Pu Hou          2016-11-11  1655  	 */
ca5955cd Pu Hou          2016-11-11  1656  	for (i = 0; i < nr_pages; i++, tail++) {
ca5955cd Pu Hou          2016-11-11  1657  		if (require_table_link(tail)) {
ca5955cd Pu Hou          2016-11-11  1658  			new = (unsigned long *) get_zeroed_page(GFP_KERNEL);
ca5955cd Pu Hou          2016-11-11  1659  			if (!new)
ca5955cd Pu Hou          2016-11-11  1660  				goto no_sdbt;
ca5955cd Pu Hou          2016-11-11  1661  			aux->sdbt_index[sfb->num_sdbt++] = (unsigned long)new;
ca5955cd Pu Hou          2016-11-11  1662  			/* Link current page to tail of chain */
ca5955cd Pu Hou          2016-11-11  1663  			*tail = (unsigned long)(void *) new + 1;
ca5955cd Pu Hou          2016-11-11  1664  			tail = new;
ca5955cd Pu Hou          2016-11-11  1665  		}
ca5955cd Pu Hou          2016-11-11  1666  		/* Tail is the entry in a SDBT */
ca5955cd Pu Hou          2016-11-11  1667  		*tail = (unsigned long)pages[i];
ca5955cd Pu Hou          2016-11-11  1668  		aux->sdb_index[i] = (unsigned long)pages[i];
ca5955cd Pu Hou          2016-11-11  1669  	}
ca5955cd Pu Hou          2016-11-11  1670  	sfb->num_sdb = nr_pages;
ca5955cd Pu Hou          2016-11-11  1671  
ca5955cd Pu Hou          2016-11-11  1672  	/* Link the last entry in the SDBT to the first SDBT */
ca5955cd Pu Hou          2016-11-11  1673  	*tail = (unsigned long) sfb->sdbt + 1;
ca5955cd Pu Hou          2016-11-11  1674  	sfb->tail = tail;
ca5955cd Pu Hou          2016-11-11  1675  
ca5955cd Pu Hou          2016-11-11  1676  	/*
ca5955cd Pu Hou          2016-11-11  1677  	 * Initial all SDBs are zeroed. Mark it as empty.
ca5955cd Pu Hou          2016-11-11  1678  	 * So there is no need to clear the full indicator
ca5955cd Pu Hou          2016-11-11  1679  	 * when this event is first added.
ca5955cd Pu Hou          2016-11-11  1680  	 */
ca5955cd Pu Hou          2016-11-11  1681  	aux->empty_mark = sfb->num_sdb - 1;
ca5955cd Pu Hou          2016-11-11  1682  
ca5955cd Pu Hou          2016-11-11  1683  	debug_sprintf_event(sfdbg, 4, "aux_buffer_setup: setup %lu SDBTs"
ca5955cd Pu Hou          2016-11-11  1684  			    " and %lu SDBs\n",
ca5955cd Pu Hou          2016-11-11  1685  			    sfb->num_sdbt, sfb->num_sdb);
ca5955cd Pu Hou          2016-11-11  1686  
ca5955cd Pu Hou          2016-11-11  1687  	return aux;
ca5955cd Pu Hou          2016-11-11  1688  
ca5955cd Pu Hou          2016-11-11  1689  no_sdbt:
ca5955cd Pu Hou          2016-11-11  1690  	/* SDBs (AUX buffer pages) are freed by caller */
ca5955cd Pu Hou          2016-11-11  1691  	for (i = 0; i < sfb->num_sdbt; i++)
ca5955cd Pu Hou          2016-11-11  1692  		free_page(aux->sdbt_index[i]);
ca5955cd Pu Hou          2016-11-11  1693  	kfree(aux->sdb_index);
ca5955cd Pu Hou          2016-11-11  1694  no_sdb_index:
ca5955cd Pu Hou          2016-11-11  1695  	kfree(aux->sdbt_index);
ca5955cd Pu Hou          2016-11-11  1696  no_sdbt_index:
ca5955cd Pu Hou          2016-11-11  1697  	kfree(aux);
ca5955cd Pu Hou          2016-11-11  1698  no_aux:
ca5955cd Pu Hou          2016-11-11  1699  	return NULL;
ca5955cd Pu Hou          2016-11-11  1700  }
ca5955cd Pu Hou          2016-11-11  1701  

:::::: The code at line 1606 was first introduced by commit
:::::: ca5955cdeae744edd3dcc65d677e833fc29658c2 s390/cpumf: introduce AUX buffer for dump diagnostic sample data

:::::: TO: Pu Hou <bjhoupu@linux.vnet.ibm.com>
:::::: CC: Martin Schwidefsky <schwidefsky@de.ibm.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Hendrik Brueckner July 3, 2018, 7:31 a.m. UTC | #2 
On Mon, Jul 02, 2018 at 04:33:28PM -0600, Mathieu Poirier wrote:
> It can be advantagous to have access to all the information conveyed by

> a perf_event when setting up the AUX buffer, as it is the case when

> dealing with PMU specific driver configuration communicated to the kernel

> using an ioctl() call.

> 

> As such simply replace the cpu information by the complete perf_event

> structure and change all affected customers.

> 

> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> ---

>  arch/s390/kernel/perf_cpum_sf.c                  | 4 ++--

>  arch/x86/events/intel/bts.c                      | 4 +++-

>  arch/x86/events/intel/pt.c                       | 5 +++--

>  drivers/hwtracing/coresight/coresight-etm-perf.c | 6 +++---

>  drivers/perf/arm_spe_pmu.c                       | 6 +++---

>  include/linux/perf_event.h                       | 2 +-

>  kernel/events/ring_buffer.c                      | 2 +-

>  7 files changed, 16 insertions(+), 13 deletions(-)

> 

> diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c

> index 0292d68e7dde..e06daaa08894 100644

> --- a/arch/s390/kernel/perf_cpum_sf.c

> +++ b/arch/s390/kernel/perf_cpum_sf.c

> @@ -1601,8 +1601,8 @@ static void aux_buffer_free(void *data)

>   *

>   * Return the private AUX buffer structure if success or NULL if fails.

>   */

> -static void *aux_buffer_setup(int cpu, void **pages, int nr_pages,

> -			      bool snapshot)

> +static void *aux_buffer_setup(struct perf_event *event, void **pages,

> +			      int nr_pages, bool snapshot);


Please remove the trailing semi-colon (;) in the function definition causing
the kbuild error.  Also, it would be great if you also could update the
function comment and replace the @cpu by the @event.

Many thanks.
Alexander Shishkin July 3, 2018, 9:27 a.m. UTC | #3 
On Mon, Jul 02, 2018 at 04:33:28PM -0600, Mathieu Poirier wrote:
> It can be advantagous to have access to all the information conveyed by

> a perf_event when setting up the AUX buffer, as it is the case when

> dealing with PMU specific driver configuration communicated to the kernel

> using an ioctl() call.

> 

> As such simply replace the cpu information by the complete perf_event

> structure and change all affected customers.


Ok, but can you provide details about which parts of perf_event you need
and how they are used? Is it the attribute? What's the ioctl command in
question? Also, should we worry that the PMU specific configuration isn't
part of the attribute? Or, if not, why? I think we talked about this
before, but I forgot and this patch needs to explain it anyway.

Thanks!
--
Alex
Mathieu Poirier July 3, 2018, 5:37 p.m. UTC | #4 
On Tue, 3 Jul 2018 at 01:31, Hendrik Brueckner <brueckner@linux.ibm.com> wrote:
>

> On Mon, Jul 02, 2018 at 04:33:28PM -0600, Mathieu Poirier wrote:

> > It can be advantagous to have access to all the information conveyed by

> > a perf_event when setting up the AUX buffer, as it is the case when

> > dealing with PMU specific driver configuration communicated to the kernel

> > using an ioctl() call.

> >

> > As such simply replace the cpu information by the complete perf_event

> > structure and change all affected customers.

> >

> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> > ---

> >  arch/s390/kernel/perf_cpum_sf.c                  | 4 ++--

> >  arch/x86/events/intel/bts.c                      | 4 +++-

> >  arch/x86/events/intel/pt.c                       | 5 +++--

> >  drivers/hwtracing/coresight/coresight-etm-perf.c | 6 +++---

> >  drivers/perf/arm_spe_pmu.c                       | 6 +++---

> >  include/linux/perf_event.h                       | 2 +-

> >  kernel/events/ring_buffer.c                      | 2 +-

> >  7 files changed, 16 insertions(+), 13 deletions(-)

> >

> > diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c

> > index 0292d68e7dde..e06daaa08894 100644

> > --- a/arch/s390/kernel/perf_cpum_sf.c

> > +++ b/arch/s390/kernel/perf_cpum_sf.c

> > @@ -1601,8 +1601,8 @@ static void aux_buffer_free(void *data)

> >   *

> >   * Return the private AUX buffer structure if success or NULL if fails.

> >   */

> > -static void *aux_buffer_setup(int cpu, void **pages, int nr_pages,

> > -                           bool snapshot)

> > +static void *aux_buffer_setup(struct perf_event *event, void **pages,

> > +                           int nr_pages, bool snapshot);

>

> Please remove the trailing semi-colon (;) in the function definition causing

> the kbuild error.  Also, it would be great if you also could update the

> function comment and replace the @cpu by the @event.


Had I realised it was just as easy to compile for s390 as it is for
ARM64, I would have done so ahead of time.  Well done on that front.
All fixed now.

Thanks,
Mathieu

>

> Many thanks.

>
diff mbox series

Patch

diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c
index 0292d68e7dde..e06daaa08894 100644
--- a/arch/s390/kernel/perf_cpum_sf.c
+++ b/arch/s390/kernel/perf_cpum_sf.c
@@ -1601,8 +1601,8 @@  static void aux_buffer_free(void *data)
  *
  * Return the private AUX buffer structure if success or NULL if fails.
  */
-static void *aux_buffer_setup(int cpu, void **pages, int nr_pages,
-			      bool snapshot)
+static void *aux_buffer_setup(struct perf_event *event, void **pages,
+			      int nr_pages, bool snapshot);
 {
 	struct sf_buffer *sfb;
 	struct aux_buffer *aux;
diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
index 24ffa1e88cf9..7139f6bf27ad 100644
--- a/arch/x86/events/intel/bts.c
+++ b/arch/x86/events/intel/bts.c
@@ -77,10 +77,12 @@  static size_t buf_size(struct page *page)
 }
 
 static void *
-bts_buffer_setup_aux(int cpu, void **pages, int nr_pages, bool overwrite)
+bts_buffer_setup_aux(struct perf_event *event, void **pages,
+		     int nr_pages, bool overwrite)
 {
 	struct bts_buffer *buf;
 	struct page *page;
+	int cpu = event->cpu;
 	int node = (cpu == -1) ? cpu : cpu_to_node(cpu);
 	unsigned long offset;
 	size_t size = nr_pages << PAGE_SHIFT;
diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 8d016ce5b80d..8f4c98fdd03c 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1104,10 +1104,11 @@  static int pt_buffer_init_topa(struct pt_buffer *buf, unsigned long nr_pages,
  * Return:	Our private PT buffer structure.
  */
 static void *
-pt_buffer_setup_aux(int cpu, void **pages, int nr_pages, bool snapshot)
+pt_buffer_setup_aux(struct perf_event *event, void **pages,
+		    int nr_pages, bool snapshot)
 {
 	struct pt_buffer *buf;
-	int node, ret;
+	int node, ret, cpu = event->cpu;
 
 	if (!nr_pages)
 		return NULL;
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 677695635211..0f5e03e4df22 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -181,15 +181,15 @@  static void etm_free_aux(void *data)
 	schedule_work(&event_data->work);
 }
 
-static void *etm_setup_aux(int event_cpu, void **pages,
+static void *etm_setup_aux(struct perf_event *event, void **pages,
 			   int nr_pages, bool overwrite)
 {
-	int cpu;
+	int cpu = event->cpu;
 	cpumask_t *mask;
 	struct coresight_device *sink;
 	struct etm_event_data *event_data = NULL;
 
-	event_data = alloc_event_data(event_cpu);
+	event_data = alloc_event_data(cpu);
 	if (!event_data)
 		return NULL;
 	INIT_WORK(&event_data->work, free_event_data);
diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 54ec278d2fc4..4dcd7bf14dcc 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -824,10 +824,10 @@  static void arm_spe_pmu_read(struct perf_event *event)
 {
 }
 
-static void *arm_spe_pmu_setup_aux(int cpu, void **pages, int nr_pages,
-				   bool snapshot)
+static void *arm_spe_pmu_setup_aux(struct perf_event *event, void **pages,
+				   int nr_pages, bool snapshot)
 {
-	int i;
+	int i, cpu = event->cpu;
 	struct page **pglist;
 	struct arm_spe_pmu_buf *buf;
 
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 1fa12887ec02..4d9c8f30ca6c 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -409,7 +409,7 @@  struct pmu {
 	/*
 	 * Set up pmu-private data structures for an AUX area
 	 */
-	void *(*setup_aux)		(int cpu, void **pages,
+	void *(*setup_aux)		(struct perf_event *event, void **pages,
 					 int nr_pages, bool overwrite);
 					/* optional */
 
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 5d3cf407e374..4c96c7575224 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -648,7 +648,7 @@  int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event,
 			goto out;
 	}
 
-	rb->aux_priv = event->pmu->setup_aux(event->cpu, rb->aux_pages, nr_pages,
+	rb->aux_priv = event->pmu->setup_aux(event, rb->aux_pages, nr_pages,
 					     overwrite);
 	if (!rb->aux_priv)
 		goto out;