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.
This patch changes the used_mask initialization to be runtime
based on the actual size of the array.
Signed-off-by: Mark Salter <[email protected]>
---
drivers/bus/arm-cci.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 84fd660..1d83072 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -679,13 +679,13 @@ static int
validate_group(struct perf_event *event)
{
struct perf_event *sibling, *leader = event->group_leader;
- struct cci_pmu_hw_events fake_pmu = {
- /*
- * Initialise the fake PMU. We only need to populate the
- * used_mask for the purposes of validation.
- */
- .used_mask = CPU_BITS_NONE,
- };
+ struct cci_pmu_hw_events fake_pmu;
+
+ /*
+ * Initialise the fake PMU. We only need to populate the
+ * used_mask for the purposes of validation.
+ */
+ memset(fake_pmu.used_mask, 0, sizeof(fake_pmu.used_mask));
if (!validate_event(&fake_pmu, leader))
return -EINVAL;
--
1.9.3
On 08/04/15 19:21, 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.
> This patch changes the used_mask initialization to be runtime
> based on the actual size of the array.
>
> Signed-off-by: Mark Salter <[email protected]>
> ---
> drivers/bus/arm-cci.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 84fd660..1d83072 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -679,13 +679,13 @@ static int
> validate_group(struct perf_event *event)
> {
> struct perf_event *sibling, *leader = event->group_leader;
> - struct cci_pmu_hw_events fake_pmu = {
> - /*
> - * Initialise the fake PMU. We only need to populate the
> - * used_mask for the purposes of validation.
> - */
> - .used_mask = CPU_BITS_NONE,
> - };
> + struct cci_pmu_hw_events fake_pmu;
> +
> + /*
> + * Initialise the fake PMU. We only need to populate the
> + * used_mask for the purposes of validation.
> + */
> + memset(fake_pmu.used_mask, 0, sizeof(fake_pmu.used_mask));
The patch looks good to me.
Reviewed-by: Suzuki K. Poulose <[email protected]>
Suzuki
On Wed, Apr 08, 2015 at 07:21:24PM +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.
Whoops. My bad.
> This patch changes the used_mask initialization to be runtime
> based on the actual size of the array.
>
> Signed-off-by: Mark Salter <[email protected]>
> ---
> drivers/bus/arm-cci.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 84fd660..1d83072 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -679,13 +679,13 @@ static int
> validate_group(struct perf_event *event)
> {
> struct perf_event *sibling, *leader = event->group_leader;
> - struct cci_pmu_hw_events fake_pmu = {
> - /*
> - * Initialise the fake PMU. We only need to populate the
> - * used_mask for the purposes of validation.
> - */
> - .used_mask = CPU_BITS_NONE,
Can we not simply change this to:
.used_mask = { 0 },
That should result in the entire array being zeroed.
Thanks,
Mark.
On Thu, 2015-04-09 at 14:51 +0100, Mark Rutland wrote:
> On Wed, Apr 08, 2015 at 07:21:24PM +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.
>
> Whoops. My bad.
>
> > This patch changes the used_mask initialization to be runtime
> > based on the actual size of the array.
> >
> > Signed-off-by: Mark Salter <[email protected]>
> > ---
> > drivers/bus/arm-cci.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> > index 84fd660..1d83072 100644
> > --- a/drivers/bus/arm-cci.c
> > +++ b/drivers/bus/arm-cci.c
> > @@ -679,13 +679,13 @@ static int
> > validate_group(struct perf_event *event)
> > {
> > struct perf_event *sibling, *leader = event->group_leader;
> > - struct cci_pmu_hw_events fake_pmu = {
> > - /*
> > - * Initialise the fake PMU. We only need to populate the
> > - * used_mask for the purposes of validation.
> > - */
> > - .used_mask = CPU_BITS_NONE,
>
> Can we not simply change this to:
>
> .used_mask = { 0 },
>
> That should result in the entire array being zeroed.
It does, but it also causes the whole struct to be cleared.
With the memset, only used_mask gets cleared.
On Thu, Apr 09, 2015 at 03:11:43PM +0100, Mark Salter wrote:
> On Thu, 2015-04-09 at 14:51 +0100, Mark Rutland wrote:
> > On Wed, Apr 08, 2015 at 07:21:24PM +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.
> >
> > Whoops. My bad.
> >
> > > This patch changes the used_mask initialization to be runtime
> > > based on the actual size of the array.
> > >
> > > Signed-off-by: Mark Salter <[email protected]>
> > > ---
> > > drivers/bus/arm-cci.c | 14 +++++++-------
> > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> > > index 84fd660..1d83072 100644
> > > --- a/drivers/bus/arm-cci.c
> > > +++ b/drivers/bus/arm-cci.c
> > > @@ -679,13 +679,13 @@ static int
> > > validate_group(struct perf_event *event)
> > > {
> > > struct perf_event *sibling, *leader = event->group_leader;
> > > - struct cci_pmu_hw_events fake_pmu = {
> > > - /*
> > > - * Initialise the fake PMU. We only need to populate the
> > > - * used_mask for the purposes of validation.
> > > - */
> > > - .used_mask = CPU_BITS_NONE,
> >
> > Can we not simply change this to:
> >
> > .used_mask = { 0 },
> >
> > That should result in the entire array being zeroed.
>
> It does, but it also causes the whole struct to be cleared.
Sure, but it's also the minimal diff, and it's easier to read. This was
what the code was intended to be initially.
> With the memset, only used_mask gets cleared.
Is there an appreciable difference between the two performance-wise?
Mark.
On Thu, 2015-04-09 at 15:20 +0100, Mark Rutland wrote:
> On Thu, Apr 09, 2015 at 03:11:43PM +0100, Mark Salter wrote:
> > On Thu, 2015-04-09 at 14:51 +0100, Mark Rutland wrote:
> > > On Wed, Apr 08, 2015 at 07:21:24PM +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.
> > >
> > > Whoops. My bad.
> > >
> > > > This patch changes the used_mask initialization to be runtime
> > > > based on the actual size of the array.
> > > >
> > > > Signed-off-by: Mark Salter <[email protected]>
> > > > ---
> > > > drivers/bus/arm-cci.c | 14 +++++++-------
> > > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> > > > index 84fd660..1d83072 100644
> > > > --- a/drivers/bus/arm-cci.c
> > > > +++ b/drivers/bus/arm-cci.c
> > > > @@ -679,13 +679,13 @@ static int
> > > > validate_group(struct perf_event *event)
> > > > {
> > > > struct perf_event *sibling, *leader = event->group_leader;
> > > > - struct cci_pmu_hw_events fake_pmu = {
> > > > - /*
> > > > - * Initialise the fake PMU. We only need to populate the
> > > > - * used_mask for the purposes of validation.
> > > > - */
> > > > - .used_mask = CPU_BITS_NONE,
> > >
> > > Can we not simply change this to:
> > >
> > > .used_mask = { 0 },
> > >
> > > That should result in the entire array being zeroed.
> >
> > It does, but it also causes the whole struct to be cleared.
>
> Sure, but it's also the minimal diff, and it's easier to read. This was
> what the code was intended to be initially.
>
> > With the memset, only used_mask gets cleared.
>
> Is there an appreciable difference between the two performance-wise?
I dunno. It is 3 strp insns vs 1 str.
If you want the static init, I'll send another patch.
> > > > > validate_group(struct perf_event *event)
> > > > > {
> > > > > struct perf_event *sibling, *leader = event->group_leader;
> > > > > - struct cci_pmu_hw_events fake_pmu = {
> > > > > - /*
> > > > > - * Initialise the fake PMU. We only need to populate the
> > > > > - * used_mask for the purposes of validation.
> > > > > - */
> > > > > - .used_mask = CPU_BITS_NONE,
> > > >
> > > > Can we not simply change this to:
> > > >
> > > > .used_mask = { 0 },
> > > >
> > > > That should result in the entire array being zeroed.
> > >
> > > It does, but it also causes the whole struct to be cleared.
> >
> > Sure, but it's also the minimal diff, and it's easier to read. This was
> > what the code was intended to be initially.
> >
> > > With the memset, only used_mask gets cleared.
> >
> > Is there an appreciable difference between the two performance-wise?
>
> I dunno. It is 3 strp insns vs 1 str.
> If you want the static init, I'll send another patch.
I'd prefer the designated initializer to the memset.
Thanks,
Mark.
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 <[email protected]>
---
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 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 <[email protected]>
Reviewed-by: Mark Rutland <[email protected]>
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 <[email protected]>
>
> Reviewed-by: Mark Rutland <[email protected]>
>
> 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
[email protected] 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 <[email protected]>
> ---
> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>>> ---
>>> 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 <[email protected]>
>>
>> Reviewed-by: Mark Rutland <[email protected]>
>>
>> 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
> [email protected] 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 <[email protected]>
> >>
> >> Reviewed-by: Mark Rutland <[email protected]>
> >>
> >> 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
> > [email protected] 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 [email protected] the correct
address? I got the cc:
From: Mark Salter <[email protected]>
To: [email protected]
Cc: Mark Salter <[email protected]>
Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
Date: Tue, 28 Apr 2015 13:09:32 -0400
Message-Id: <[email protected]>
> > 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 [email protected] the correct
> address? I got the cc:
>
> From: Mark Salter <[email protected]>
> To: [email protected]
> Cc: Mark Salter <[email protected]>
> Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
> Date: Tue, 28 Apr 2015 13:09:32 -0400
> Message-Id: <[email protected]>
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 [email protected] the correct
> > address? I got the cc:
> >
> > From: Mark Salter <[email protected]>
> > To: [email protected]
> > Cc: Mark Salter <[email protected]>
> > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
> > Date: Tue, 28 Apr 2015 13:09:32 -0400
> > Message-Id: <[email protected]>
>
> 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 [email protected] the correct
> > > address? I got the cc:
> > >
> > > From: Mark Salter <[email protected]>
> > > To: [email protected]
> > > Cc: Mark Salter <[email protected]>
> > > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
> > > Date: Tue, 28 Apr 2015 13:09:32 -0400
> > > Message-Id: <[email protected]>
> >
> > 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 [email protected] 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 [email protected] the correct
> > > > address? I got the cc:
> > > >
> > > > From: Mark Salter <[email protected]>
> > > > To: [email protected]
> > > > Cc: Mark Salter <[email protected]>
> > > > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
> > > > Date: Tue, 28 Apr 2015 13:09:32 -0400
> > > > Message-Id: <[email protected]>
> > >
> > > 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 [email protected] 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 [email protected] the correct
> > > > > address? I got the cc:
> > > > >
> > > > > From: Mark Salter <[email protected]>
> > > > > To: [email protected]
> > > > > Cc: Mark Salter <[email protected]>
> > > > > Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
> > > > > Date: Tue, 28 Apr 2015 13:09:32 -0400
> > > > > Message-Id: <[email protected]>
> > > >
> > > > 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 [email protected] 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 [email protected] 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