| Message ID | 1428591425-17330-1-git-send-email-msalter@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 09, 2015 at 03:57:05PM +0100, Mark Salter wrote: > Currently in validate_group(), there is a static initializer > for fake_pmu.used_mask which is based on CPU_BITS_NONE but > the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. > CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE > is not correct and will cause a build failure if NR_CPUS > is set high enough to make CPU_BITS_NONE larger than used_mask. > > Signed-off-by: Mark Salter <msalter@redhat.com> Reviewed-by: Mark Rutland <mark.rutland@arm.com> Arnd, Olof, are you happy to take this via arm-soc? My (broken) patch went via Will's tree because of a perf dependency, but other CCI patches have gone via you guys. Thanks, Mark. > --- > drivers/bus/arm-cci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c > index 84fd660..759e41c 100644 > --- a/drivers/bus/arm-cci.c > +++ b/drivers/bus/arm-cci.c > @@ -684,7 +684,7 @@ validate_group(struct perf_event *event) > * Initialise the fake PMU. We only need to populate the > * used_mask for the purposes of validation. > */ > - .used_mask = CPU_BITS_NONE, > + .used_mask = { 0 }, > }; > > if (!validate_event(&fake_pmu, leader)) > -- > 1.8.3.1 >
On Thu, Apr 09, 2015 at 04:36:29PM +0100, Mark Rutland wrote: > On Thu, Apr 09, 2015 at 03:57:05PM +0100, Mark Salter wrote: > > Currently in validate_group(), there is a static initializer > > for fake_pmu.used_mask which is based on CPU_BITS_NONE but > > the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. > > CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE > > is not correct and will cause a build failure if NR_CPUS > > is set high enough to make CPU_BITS_NONE larger than used_mask. > > > > Signed-off-by: Mark Salter <msalter@redhat.com> > > Reviewed-by: Mark Rutland <mark.rutland@arm.com> > > Arnd, Olof, are you happy to take this via arm-soc? > > My (broken) patch went via Will's tree because of a perf dependency, but > other CCI patches have gone via you guys. Yeah, arm-soc is the best place for this. You should resend to arm@kernel.org as a new patch with the relevant acks. Will
On 09/04/15 15:57, Mark Salter wrote: > Currently in validate_group(), there is a static initializer > for fake_pmu.used_mask which is based on CPU_BITS_NONE but > the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. > CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE > is not correct and will cause a build failure if NR_CPUS > is set high enough to make CPU_BITS_NONE larger than used_mask. > > Signed-off-by: Mark Salter <msalter@redhat.com> > --- > drivers/bus/arm-cci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c > index 84fd660..759e41c 100644 > --- a/drivers/bus/arm-cci.c > +++ b/drivers/bus/arm-cci.c > @@ -684,7 +684,7 @@ validate_group(struct perf_event *event) > * Initialise the fake PMU. We only need to populate the > * used_mask for the purposes of validation. > */ > - .used_mask = CPU_BITS_NONE, > + .used_mask = { 0 }, > }; > > if (!validate_event(&fake_pmu, leader)) > I have a series of patches to add CCI-500 PMU, targeting 4.2, which changes the used_mask to a pointer and changes it to depend on the number of counters available on the CCI PMU. I will post it in very soon here, which could make this patch obsolete. Let me know if you don't mind getting this sorted out through my series. I am fine either way. Regards Suzuki
On Wed, Apr 15, 2015 at 11:44:06AM +0100, Suzuki K. Poulose wrote: > On 09/04/15 15:57, Mark Salter wrote: > > Currently in validate_group(), there is a static initializer > > for fake_pmu.used_mask which is based on CPU_BITS_NONE but > > the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. > > CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE > > is not correct and will cause a build failure if NR_CPUS > > is set high enough to make CPU_BITS_NONE larger than used_mask. > > > > Signed-off-by: Mark Salter <msalter@redhat.com> > > --- > > drivers/bus/arm-cci.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c > > index 84fd660..759e41c 100644 > > --- a/drivers/bus/arm-cci.c > > +++ b/drivers/bus/arm-cci.c > > @@ -684,7 +684,7 @@ validate_group(struct perf_event *event) > > * Initialise the fake PMU. We only need to populate the > > * used_mask for the purposes of validation. > > */ > > - .used_mask = CPU_BITS_NONE, > > + .used_mask = { 0 }, > > }; > > > > if (!validate_event(&fake_pmu, leader)) > > > I have a series of patches to add CCI-500 PMU, targeting 4.2, which > changes the used_mask to a pointer and changes it to depend on the > number of counters available on the CCI PMU. I will post it in very > soon here, which could make this patch obsolete. Let me know if you > don't mind getting this sorted out through my series. I am fine either way. I'd rather have the fix in for 4.1 and then have your CCI-500 patches based on that. Will
On 15/04/15 12:58, Will Deacon wrote: > On Wed, Apr 15, 2015 at 11:44:06AM +0100, Suzuki K. Poulose wrote: >> On 09/04/15 15:57, Mark Salter wrote: >>> Currently in validate_group(), there is a static initializer >>> for fake_pmu.used_mask which is based on CPU_BITS_NONE but >>> the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. >>> CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE >>> is not correct and will cause a build failure if NR_CPUS >>> is set high enough to make CPU_BITS_NONE larger than used_mask. >>> >>> Signed-off-by: Mark Salter <msalter@redhat.com> >>> --- >>> drivers/bus/arm-cci.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c >>> index 84fd660..759e41c 100644 >>> --- a/drivers/bus/arm-cci.c >>> +++ b/drivers/bus/arm-cci.c >>> @@ -684,7 +684,7 @@ validate_group(struct perf_event *event) >>> * Initialise the fake PMU. We only need to populate the >>> * used_mask for the purposes of validation. >>> */ >>> - .used_mask = CPU_BITS_NONE, >>> + .used_mask = { 0 }, >>> }; >>> >>> if (!validate_event(&fake_pmu, leader)) >>> >> I have a series of patches to add CCI-500 PMU, targeting 4.2, which >> changes the used_mask to a pointer and changes it to depend on the >> number of counters available on the CCI PMU. I will post it in very >> soon here, which could make this patch obsolete. Let me know if you >> don't mind getting this sorted out through my series. I am fine either way. > > I'd rather have the fix in for 4.1 and then have your CCI-500 patches based > on that. > > Will > OK, will rebase my patches on top of this one. Thanks Suzuki
On 13/04/15 13:41, Will Deacon wrote: > On Thu, Apr 09, 2015 at 04:36:29PM +0100, Mark Rutland wrote: >> On Thu, Apr 09, 2015 at 03:57:05PM +0100, Mark Salter wrote: >>> Currently in validate_group(), there is a static initializer >>> for fake_pmu.used_mask which is based on CPU_BITS_NONE but >>> the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. >>> CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE >>> is not correct and will cause a build failure if NR_CPUS >>> is set high enough to make CPU_BITS_NONE larger than used_mask. >>> >>> Signed-off-by: Mark Salter <msalter@redhat.com> >> >> Reviewed-by: Mark Rutland <mark.rutland@arm.com> >> >> Arnd, Olof, are you happy to take this via arm-soc? >> >> My (broken) patch went via Will's tree because of a perf dependency, but >> other CCI patches have gone via you guys. > > Yeah, arm-soc is the best place for this. You should resend to > arm@kernel.org as a new patch with the relevant acks. Mark Could you please send this to arm-soc as suggested by Will, with the relevant acks/reviews ? Thanks Suzuki
On Thu, 2015-04-30 at 11:55 +0100, Suzuki K. Poulose wrote: > On 13/04/15 13:41, Will Deacon wrote: > > On Thu, Apr 09, 2015 at 04:36:29PM +0100, Mark Rutland wrote: > >> On Thu, Apr 09, 2015 at 03:57:05PM +0100, Mark Salter wrote: > >>> Currently in validate_group(), there is a static initializer > >>> for fake_pmu.used_mask which is based on CPU_BITS_NONE but > >>> the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. > >>> CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE > >>> is not correct and will cause a build failure if NR_CPUS > >>> is set high enough to make CPU_BITS_NONE larger than used_mask. > >>> > >>> Signed-off-by: Mark Salter <msalter@redhat.com> > >> > >> Reviewed-by: Mark Rutland <mark.rutland@arm.com> > >> > >> Arnd, Olof, are you happy to take this via arm-soc? > >> > >> My (broken) patch went via Will's tree because of a perf dependency, but > >> other CCI patches have gone via you guys. > > > > Yeah, arm-soc is the best place for this. You should resend to > > arm@kernel.org as a new patch with the relevant acks. > Mark > > Could you please send this to arm-soc as suggested by Will, with the > relevant acks/reviews ? > I sent it on Tuesday. Did it not show up? Is arm@kernel.org the correct address? I got the cc: From: Mark Salter <msalter@redhat.com> To: arm@kernel.org Cc: Mark Salter <msalter@redhat.com> Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group() Date: Tue, 28 Apr 2015 13:09:32 -0400 Message-Id: <1430240972-16386-1-git-send-email-msalter@redhat.com>
> > Could you please send this to arm-soc as suggested by Will, with the > > relevant acks/reviews ? > > > > I sent it on Tuesday. Did it not show up? Is arm@kernel.org the correct > address? I got the cc: > > From: Mark Salter <msalter@redhat.com> > To: arm@kernel.org > Cc: Mark Salter <msalter@redhat.com> > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group() > Date: Tue, 28 Apr 2015 13:09:32 -0400 > Message-Id: <1430240972-16386-1-git-send-email-msalter@redhat.com> That's the right address, but that only goes to the maintainers, and doesn't get copied to any list. In future, please Cc linux-arm-kernel in addition. Thanks, Mark.
On Thu, 2015-04-30 at 14:33 +0100, Mark Rutland wrote: > > > Could you please send this to arm-soc as suggested by Will, with the > > > relevant acks/reviews ? > > > > > > > I sent it on Tuesday. Did it not show up? Is arm@kernel.org the correct > > address? I got the cc: > > > > From: Mark Salter <msalter@redhat.com> > > To: arm@kernel.org > > Cc: Mark Salter <msalter@redhat.com> > > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group() > > Date: Tue, 28 Apr 2015 13:09:32 -0400 > > Message-Id: <1430240972-16386-1-git-send-email-msalter@redhat.com> > > That's the right address, but that only goes to the maintainers, and > doesn't get copied to any list. In future, please Cc linux-arm-kernel in > addition. That's where I sent it originally.
On Thu, Apr 30, 2015 at 03:03:07PM +0100, Mark Salter wrote: > On Thu, 2015-04-30 at 14:33 +0100, Mark Rutland wrote: > > > > Could you please send this to arm-soc as suggested by Will, with the > > > > relevant acks/reviews ? > > > > > > > > > > I sent it on Tuesday. Did it not show up? Is arm@kernel.org the correct > > > address? I got the cc: > > > > > > From: Mark Salter <msalter@redhat.com> > > > To: arm@kernel.org > > > Cc: Mark Salter <msalter@redhat.com> > > > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group() > > > Date: Tue, 28 Apr 2015 13:09:32 -0400 > > > Message-Id: <1430240972-16386-1-git-send-email-msalter@redhat.com> > > > > That's the right address, but that only goes to the maintainers, and > > doesn't get copied to any list. In future, please Cc linux-arm-kernel in > > addition. > > That's where I sent it originally. Sure, but it's good to Cc when sending to arm-soc so as to make it visible that the patches have been sent. Doing so avoids the necessity of queries like Suzuki's, and makes it possible for others to reply to the version sent to arm@kernel.org in the case of conflicts or other issues. Thanks, Mark.
On Thu, 2015-04-30 at 15:38 +0100, Mark Rutland wrote: > On Thu, Apr 30, 2015 at 03:03:07PM +0100, Mark Salter wrote: > > On Thu, 2015-04-30 at 14:33 +0100, Mark Rutland wrote: > > > > > Could you please send this to arm-soc as suggested by Will, with the > > > > > relevant acks/reviews ? > > > > > > > > > > > > > I sent it on Tuesday. Did it not show up? Is arm@kernel.org the correct > > > > address? I got the cc: > > > > > > > > From: Mark Salter <msalter@redhat.com> > > > > To: arm@kernel.org > > > > Cc: Mark Salter <msalter@redhat.com> > > > > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group() > > > > Date: Tue, 28 Apr 2015 13:09:32 -0400 > > > > Message-Id: <1430240972-16386-1-git-send-email-msalter@redhat.com> > > > > > > That's the right address, but that only goes to the maintainers, and > > > doesn't get copied to any list. In future, please Cc linux-arm-kernel in > > > addition. > > > > That's where I sent it originally. > > Sure, but it's good to Cc when sending to arm-soc so as to make it > visible that the patches have been sent. Doing so avoids the necessity > of queries like Suzuki's, and makes it possible for others to reply to > the version sent to arm@kernel.org in the case of conflicts or other > issues. But why did it need to be sent to a private maintainer's list in the first place? I think that the destination addresses of the original posting was perfectly reasonable given output from get_maintainer.pl and that sending me to a private list was an unnecessary hoop to jump through.
On Thursday 30 April 2015 10:46:13 Mark Salter wrote: > On Thu, 2015-04-30 at 15:38 +0100, Mark Rutland wrote: > > On Thu, Apr 30, 2015 at 03:03:07PM +0100, Mark Salter wrote: > > > On Thu, 2015-04-30 at 14:33 +0100, Mark Rutland wrote: > > > > > > Could you please send this to arm-soc as suggested by Will, with the > > > > > > relevant acks/reviews ? > > > > > > > > > > > > > > > > I sent it on Tuesday. Did it not show up? Is arm@kernel.org the correct > > > > > address? I got the cc: > > > > > > > > > > From: Mark Salter <msalter@redhat.com> > > > > > To: arm@kernel.org > > > > > Cc: Mark Salter <msalter@redhat.com> > > > > > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group() > > > > > Date: Tue, 28 Apr 2015 13:09:32 -0400 > > > > > Message-Id: <1430240972-16386-1-git-send-email-msalter@redhat.com> > > > > > > > > That's the right address, but that only goes to the maintainers, and > > > > doesn't get copied to any list. In future, please Cc linux-arm-kernel in > > > > addition. > > > > > > That's where I sent it originally. > > > > Sure, but it's good to Cc when sending to arm-soc so as to make it > > visible that the patches have been sent. Doing so avoids the necessity > > of queries like Suzuki's, and makes it possible for others to reply to > > the version sent to arm@kernel.org in the case of conflicts or other > > issues. > > But why did it need to be sent to a private maintainer's list in the > first place? I think that the destination addresses of the original > posting was perfectly reasonable given output from get_maintainer.pl > and that sending me to a private list was an unnecessary hoop to > jump through. The purpose of the arm@kernel.org alias is for subarch maintainers to send us stuff, it's not really meant for normal developers, unless specifically advised by a maintainer. Each file we maintain through arm-soc normally belongs to one subarch, so we tend to not pick up any patches on the mailing list and instead wait for that subarch maintainer to pick them up and forward the changes to us. That model model breaks down to some degree for drivers/bus, in particular for stuff that is not specific to just one SoC. I have the patch in my todo list now, sorry about missing that earlier. We should probably come up with a better way to handle patches like this one. Arnd
diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c index 84fd660..759e41c 100644 --- a/drivers/bus/arm-cci.c +++ b/drivers/bus/arm-cci.c @@ -684,7 +684,7 @@ validate_group(struct perf_event *event) * Initialise the fake PMU. We only need to populate the * used_mask for the purposes of validation. */ - .used_mask = CPU_BITS_NONE, + .used_mask = { 0 }, }; if (!validate_event(&fake_pmu, leader))
Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask. Signed-off-by: Mark Salter <msalter@redhat.com> --- drivers/bus/arm-cci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)