Re: [PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
Jiri Olsa [jo...@redhat.com] wrote:
| On Tue, Dec 18, 2012 at 11:28:02PM -0800, Sukadev Bhattiprolu wrote:
| >
| > Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
| > Make them global so they are available to all architectures.
| >
| > Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
| > variable name as a parameter.
| >
| hi,
| the change looks ok apart from some nits below.
|
| There' another version of the x86 event attributes change
| I mentioned earlier:
|
| http://marc.info/?l=linux-kernel=135601815224373=2
|
| I'm not sure which one will make it in first, but you
| guys need to sync ;-) CC-ing Andi and Stephane.
One change that would help powerpc (and other architectures) is to move
the 'struct perf_pmu_events_attr' to say, include/linux/perf_event.h.
Each architecture can define EVENT_VAR(), EVENT_PTR() etc as needed.
|
| thanks,
| jirka
|
| > +struct perf_pmu_events_attr {
| > + struct device_attribute attr;
| > + u64 id;
| > +};
| > +
| > +#define PMU_EVENT_PTR(_var)&_var.attr.attr
|
| this one seems superfluous as well, could be replaced by '&'
I guess that would encode the assumption that both the 'attr' fields are
the first in their respective structures. If so, an explicit comment beside
the fields would be useful.
Sukadev
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
Jiri Olsa [jo...@redhat.com] wrote:
| On Tue, Dec 18, 2012 at 11:28:02PM -0800, Sukadev Bhattiprolu wrote:
|
| Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
| Make them global so they are available to all architectures.
|
| Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
| variable name as a parameter.
|
| hi,
| the change looks ok apart from some nits below.
|
| There' another version of the x86 event attributes change
| I mentioned earlier:
|
| http://marc.info/?l=linux-kernelm=135601815224373w=2
|
| I'm not sure which one will make it in first, but you
| guys need to sync ;-) CC-ing Andi and Stephane.
One change that would help powerpc (and other architectures) is to move
the 'struct perf_pmu_events_attr' to say, include/linux/perf_event.h.
Each architecture can define EVENT_VAR(), EVENT_PTR() etc as needed.
|
| thanks,
| jirka
|
snip
| +struct perf_pmu_events_attr {
| + struct device_attribute attr;
| + u64 id;
| +};
| +
| +#define PMU_EVENT_PTR(_var)_var.attr.attr
|
| this one seems superfluous as well, could be replaced by ''
I guess that would encode the assumption that both the 'attr' fields are
the first in their respective structures. If so, an explicit comment beside
the fields would be useful.
Sukadev
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
On Tue, Dec 18, 2012 at 11:28:02PM -0800, Sukadev Bhattiprolu wrote:
>
> Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
> Make them global so they are available to all architectures.
>
> Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
> variable name as a parameter.
>
hi,
the change looks ok apart from some nits below.
There' another version of the x86 event attributes change
I mentioned earlier:
http://marc.info/?l=linux-kernel=135601815224373=2
I'm not sure which one will make it in first, but you
guys need to sync ;-) CC-ing Andi and Stephane.
thanks,
jirka
> Signed-off-by: Sukadev Bhattiprolu
> ---
> arch/x86/kernel/cpu/perf_event.c | 17 +
> include/linux/perf_event.h | 13 +
> 2 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event.c
> b/arch/x86/kernel/cpu/perf_event.c
> index 4428fd1..24bc505 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -1316,11 +1316,6 @@ static struct attribute_group x86_pmu_format_group = {
> .attrs = NULL,
> };
>
> -struct perf_pmu_events_attr {
> - struct device_attribute attr;
> - u64 id;
> -};
> -
> /*
> * Remove all undefined events (x86_pmu.event_map(id) == 0)
> * out of events_attr attributes.
> @@ -1351,14 +1346,12 @@ static ssize_t events_sysfs_show(struct device *dev,
> struct device_attribute *at
> return x86_pmu.events_sysfs_show(page, config);
> }
>
> -#define EVENT_VAR(_id) event_attr_##_id
> -#define EVENT_PTR(_id) _attr_##_id.attr.attr
> +#define EVENT_VAR(_id) event_attr_##_id
> +#define EVENT_ID(_id)PERF_COUNT_HW_##_id
> +#define EVENT_PTR(_id) PMU_EVENT_PTR(EVENT_VAR(_id))
>
> -#define EVENT_ATTR(_name, _id) \
> -static struct perf_pmu_events_attr EVENT_VAR(_id) = {\
> - .attr = __ATTR(_name, 0444, events_sysfs_show, NULL), \
> - .id = PERF_COUNT_HW_##_id, \
> -};
> +#define EVENT_ATTR(_name, _id)
> \
> + PMU_EVENT_ATTR(_name, EVENT_VAR(_id), EVENT_ID(_id), events_sysfs_show)
probably no need to define EVENT_ID macro if it's used on just one place
>
> EVENT_ATTR(cpu-cycles, CPU_CYCLES );
> EVENT_ATTR(instructions, INSTRUCTIONS);
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 6bfb2fa..31692cb 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -817,6 +817,19 @@ do {
> \
> } while (0)
>
>
> +struct perf_pmu_events_attr {
> + struct device_attribute attr;
> + u64 id;
> +};
> +
> +#define PMU_EVENT_PTR(_var) &_var.attr.attr
this one seems superfluous as well, could be replaced by '&'
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
On Tue, Dec 18, 2012 at 11:28:02PM -0800, Sukadev Bhattiprolu wrote:
Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
Make them global so they are available to all architectures.
Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
variable name as a parameter.
hi,
the change looks ok apart from some nits below.
There' another version of the x86 event attributes change
I mentioned earlier:
http://marc.info/?l=linux-kernelm=135601815224373w=2
I'm not sure which one will make it in first, but you
guys need to sync ;-) CC-ing Andi and Stephane.
thanks,
jirka
Signed-off-by: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
---
arch/x86/kernel/cpu/perf_event.c | 17 +
include/linux/perf_event.h | 13 +
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c
b/arch/x86/kernel/cpu/perf_event.c
index 4428fd1..24bc505 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1316,11 +1316,6 @@ static struct attribute_group x86_pmu_format_group = {
.attrs = NULL,
};
-struct perf_pmu_events_attr {
- struct device_attribute attr;
- u64 id;
-};
-
/*
* Remove all undefined events (x86_pmu.event_map(id) == 0)
* out of events_attr attributes.
@@ -1351,14 +1346,12 @@ static ssize_t events_sysfs_show(struct device *dev,
struct device_attribute *at
return x86_pmu.events_sysfs_show(page, config);
}
-#define EVENT_VAR(_id) event_attr_##_id
-#define EVENT_PTR(_id) event_attr_##_id.attr.attr
+#define EVENT_VAR(_id) event_attr_##_id
+#define EVENT_ID(_id)PERF_COUNT_HW_##_id
+#define EVENT_PTR(_id) PMU_EVENT_PTR(EVENT_VAR(_id))
-#define EVENT_ATTR(_name, _id) \
-static struct perf_pmu_events_attr EVENT_VAR(_id) = {\
- .attr = __ATTR(_name, 0444, events_sysfs_show, NULL), \
- .id = PERF_COUNT_HW_##_id, \
-};
+#define EVENT_ATTR(_name, _id)
\
+ PMU_EVENT_ATTR(_name, EVENT_VAR(_id), EVENT_ID(_id), events_sysfs_show)
probably no need to define EVENT_ID macro if it's used on just one place
EVENT_ATTR(cpu-cycles, CPU_CYCLES );
EVENT_ATTR(instructions, INSTRUCTIONS);
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 6bfb2fa..31692cb 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -817,6 +817,19 @@ do {
\
} while (0)
+struct perf_pmu_events_attr {
+ struct device_attribute attr;
+ u64 id;
+};
+
+#define PMU_EVENT_PTR(_var) _var.attr.attr
this one seems superfluous as well, could be replaced by ''
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
Make them global so they are available to all architectures.
Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
variable name as a parameter.
Signed-off-by: Sukadev Bhattiprolu
---
arch/x86/kernel/cpu/perf_event.c | 17 +
include/linux/perf_event.h | 13 +
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 4428fd1..24bc505 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1316,11 +1316,6 @@ static struct attribute_group x86_pmu_format_group = {
.attrs = NULL,
};
-struct perf_pmu_events_attr {
- struct device_attribute attr;
- u64 id;
-};
-
/*
* Remove all undefined events (x86_pmu.event_map(id) == 0)
* out of events_attr attributes.
@@ -1351,14 +1346,12 @@ static ssize_t events_sysfs_show(struct device *dev,
struct device_attribute *at
return x86_pmu.events_sysfs_show(page, config);
}
-#define EVENT_VAR(_id) event_attr_##_id
-#define EVENT_PTR(_id) _attr_##_id.attr.attr
+#define EVENT_VAR(_id) event_attr_##_id
+#define EVENT_ID(_id) PERF_COUNT_HW_##_id
+#define EVENT_PTR(_id) PMU_EVENT_PTR(EVENT_VAR(_id))
-#define EVENT_ATTR(_name, _id) \
-static struct perf_pmu_events_attr EVENT_VAR(_id) = { \
- .attr = __ATTR(_name, 0444, events_sysfs_show, NULL), \
- .id = PERF_COUNT_HW_##_id, \
-};
+#define EVENT_ATTR(_name, _id) \
+ PMU_EVENT_ATTR(_name, EVENT_VAR(_id), EVENT_ID(_id), events_sysfs_show)
EVENT_ATTR(cpu-cycles, CPU_CYCLES );
EVENT_ATTR(instructions, INSTRUCTIONS);
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 6bfb2fa..31692cb 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -817,6 +817,19 @@ do {
\
} while (0)
+struct perf_pmu_events_attr {
+ struct device_attribute attr;
+ u64 id;
+};
+
+#define PMU_EVENT_PTR(_var)&_var.attr.attr
+
+#define PMU_EVENT_ATTR(_name, _var, _id, _show)
\
+static struct perf_pmu_events_attr _var = {\
+ .attr = __ATTR(_name, 0444, _show, NULL), \
+ .id = _id, \
+};
+
#define PMU_FORMAT_ATTR(_name, _format)
\
static ssize_t \
_name##_show(struct device *dev, \
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/5] perf: Make EVENT_ATTR and EVENT_PTR global
Rename EVENT_ATTR() and EVENT_PTR() PMU_EVENT_ATTR() and PMU_EVENT_PTR().
Make them global so they are available to all architectures.
Further to allow architectures flexibility, have PMU_EVENT_PTR() pass in the
variable name as a parameter.
Signed-off-by: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
---
arch/x86/kernel/cpu/perf_event.c | 17 +
include/linux/perf_event.h | 13 +
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 4428fd1..24bc505 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1316,11 +1316,6 @@ static struct attribute_group x86_pmu_format_group = {
.attrs = NULL,
};
-struct perf_pmu_events_attr {
- struct device_attribute attr;
- u64 id;
-};
-
/*
* Remove all undefined events (x86_pmu.event_map(id) == 0)
* out of events_attr attributes.
@@ -1351,14 +1346,12 @@ static ssize_t events_sysfs_show(struct device *dev,
struct device_attribute *at
return x86_pmu.events_sysfs_show(page, config);
}
-#define EVENT_VAR(_id) event_attr_##_id
-#define EVENT_PTR(_id) event_attr_##_id.attr.attr
+#define EVENT_VAR(_id) event_attr_##_id
+#define EVENT_ID(_id) PERF_COUNT_HW_##_id
+#define EVENT_PTR(_id) PMU_EVENT_PTR(EVENT_VAR(_id))
-#define EVENT_ATTR(_name, _id) \
-static struct perf_pmu_events_attr EVENT_VAR(_id) = { \
- .attr = __ATTR(_name, 0444, events_sysfs_show, NULL), \
- .id = PERF_COUNT_HW_##_id, \
-};
+#define EVENT_ATTR(_name, _id) \
+ PMU_EVENT_ATTR(_name, EVENT_VAR(_id), EVENT_ID(_id), events_sysfs_show)
EVENT_ATTR(cpu-cycles, CPU_CYCLES );
EVENT_ATTR(instructions, INSTRUCTIONS);
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 6bfb2fa..31692cb 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -817,6 +817,19 @@ do {
\
} while (0)
+struct perf_pmu_events_attr {
+ struct device_attribute attr;
+ u64 id;
+};
+
+#define PMU_EVENT_PTR(_var)_var.attr.attr
+
+#define PMU_EVENT_ATTR(_name, _var, _id, _show)
\
+static struct perf_pmu_events_attr _var = {\
+ .attr = __ATTR(_name, 0444, _show, NULL), \
+ .id = _id, \
+};
+
#define PMU_FORMAT_ATTR(_name, _format)
\
static ssize_t \
_name##_show(struct device *dev, \
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
