[PATCH] bus: arm-cci: constify attribute_group structures.

[PATCH] bus: arm-cci: constify attribute_group structures.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by <linux/sysfs.h> work with const
attribute_group. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/bus/arm-cci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index c49da15..64c75d2 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -1445,16 +1445,16 @@ static ssize_t pmu_cpumask_attr_show(struct device *dev,
 	NULL,
 };
 
-static struct attribute_group pmu_attr_group = {
+static const struct attribute_group pmu_attr_group = {
 	.attrs = pmu_attrs,
 };
 
-static struct attribute_group pmu_format_attr_group = {
+static const struct attribute_group pmu_format_attr_group = {
 	.name = "format",
 	.attrs = NULL,		/* Filled in cci_pmu_init_attrs */
 };
 
-static struct attribute_group pmu_event_attr_group = {
+static const struct attribute_group pmu_event_attr_group = {
 	.name = "events",
 	.attrs = NULL,		/* Filled in cci_pmu_init_attrs */
 };
-- 
1.9.1

Re: [PATCH] bus: arm-cci: constify attribute_group structures.

[Mark Rutland]

On Mon, Jul 03, 2017 at 01:16:55PM +0530, Arvind Yadav wrote:
> attribute_groups are not supposed to change at runtime. All functions
> working with attribute_groups provided by <linux/sysfs.h> work with const
> attribute_group. So mark the non-const structs as const.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>

>From a quick scan, this looks sane to me:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Suzuki, Punit, I guess one of you will queue this, assuming you're happy
with it.

Mark.

> ---
>  drivers/bus/arm-cci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index c49da15..64c75d2 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -1445,16 +1445,16 @@ static ssize_t pmu_cpumask_attr_show(struct device *dev,
>  	NULL,
>  };
>  
> -static struct attribute_group pmu_attr_group = {
> +static const struct attribute_group pmu_attr_group = {
>  	.attrs = pmu_attrs,
>  };
>  
> -static struct attribute_group pmu_format_attr_group = {
> +static const struct attribute_group pmu_format_attr_group = {
>  	.name = "format",
>  	.attrs = NULL,		/* Filled in cci_pmu_init_attrs */
>  };
>  
> -static struct attribute_group pmu_event_attr_group = {
> +static const struct attribute_group pmu_event_attr_group = {
>  	.name = "events",
>  	.attrs = NULL,		/* Filled in cci_pmu_init_attrs */
>  };
> -- 
> 1.9.1
> 

Re: [PATCH] bus: arm-cci: constify attribute_group structures.

[Suzuki K Poulose]

On 03/07/17 08:46, Arvind Yadav wrote:
> attribute_groups are not supposed to change at runtime. All functions
> working with attribute_groups provided by <linux/sysfs.h> work with const
> attribute_group. So mark the non-const structs as const.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
>  drivers/bus/arm-cci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index c49da15..64c75d2 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -1445,16 +1445,16 @@ static ssize_t pmu_cpumask_attr_show(struct device *dev,
>  	NULL,
>  };
>
> -static struct attribute_group pmu_attr_group = {
> +static const struct attribute_group pmu_attr_group = {
>  	.attrs = pmu_attrs,
>  };
>
> -static struct attribute_group pmu_format_attr_group = {
> +static const struct attribute_group pmu_format_attr_group = {
>  	.name = "format",
>  	.attrs = NULL,		/* Filled in cci_pmu_init_attrs */
>  };



>
> -static struct attribute_group pmu_event_attr_group = {
> +static const struct attribute_group pmu_event_attr_group = {
>  	.name = "events",
>  	.attrs = NULL,		/* Filled in cci_pmu_init_attrs */
>  };
>

I think we cannot make these const, as the attrs field gets filled in at runtime, indicated
by the comments next to them.

Cheers
Suzuki

Re: [PATCH] bus: arm-cci: constify attribute_group structures.

[Arvind Yadav]

Hi,


On Wednesday 05 July 2017 09:33 PM, Suzuki K Poulose wrote:
> On 03/07/17 08:46, Arvind Yadav wrote:
>> attribute_groups are not supposed to change at runtime. All functions
>> working with attribute_groups provided by <linux/sysfs.h> work with 
>> const
>> attribute_group. So mark the non-const structs as const.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> ---
>>  drivers/bus/arm-cci.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
>> index c49da15..64c75d2 100644
>> --- a/drivers/bus/arm-cci.c
>> +++ b/drivers/bus/arm-cci.c
>> @@ -1445,16 +1445,16 @@ static ssize_t pmu_cpumask_attr_show(struct 
>> device *dev,
>>      NULL,
>>  };
>>
>> -static struct attribute_group pmu_attr_group = {
>> +static const struct attribute_group pmu_attr_group = {
>>      .attrs = pmu_attrs, /*This can be const*/
>>  };
>>
>> -static struct attribute_group pmu_format_attr_group = {
>> +static const struct attribute_group pmu_format_attr_group = {
>>      .name = "format",
>>      .attrs = NULL,        /* Filled in cci_pmu_init_attrs */
>>  };
>
>
>
>>
>> -static struct attribute_group pmu_event_attr_group = {
>> +static const struct attribute_group pmu_event_attr_group = {
>>      .name = "events",
>>      .attrs = NULL,        /* Filled in cci_pmu_init_attrs */
>>  };
>>
>
> I think we cannot make these const, as the attrs field gets filled in 
> at runtime, indicated
> by the comments next to them.
>
     Yes, Your are right. It's filled at run time.
         pmu_event_attr_group.attrs = model->event_attrs;
         pmu_format_attr_group.attrs = model->format_attrs;
     can we make pmu_attr_group as const.? It's already initialized.
     If you are ok with this please let me know. I can push updated patch.


> Cheers
> Suzuki
>
Thanks,
Arvind Yadav

Re: [PATCH] bus: arm-cci: constify attribute_group structures.

[Suzuki K Poulose]

On 06/07/17 05:53, Arvind Yadav wrote:
> Hi,
>
>
> On Wednesday 05 July 2017 09:33 PM, Suzuki K Poulose wrote:
>> On 03/07/17 08:46, Arvind Yadav wrote:
>>> attribute_groups are not supposed to change at runtime. All functions
>>> working with attribute_groups provided by <linux/sysfs.h> work with const
>>> attribute_group. So mark the non-const structs as const.
>>>
>>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>>> ---
>>>  drivers/bus/arm-cci.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
>>> index c49da15..64c75d2 100644
>>> --- a/drivers/bus/arm-cci.c
>>> +++ b/drivers/bus/arm-cci.c
>>> @@ -1445,16 +1445,16 @@ static ssize_t pmu_cpumask_attr_show(struct device *dev,
>>>      NULL,
>>>  };
>>>
>>> -static struct attribute_group pmu_attr_group = {
>>> +static const struct attribute_group pmu_attr_group = {
>>>      .attrs = pmu_attrs, /*This can be const*/
>>>  };
>>>
>>> -static struct attribute_group pmu_format_attr_group = {
>>> +static const struct attribute_group pmu_format_attr_group = {
>>>      .name = "format",
>>>      .attrs = NULL,        /* Filled in cci_pmu_init_attrs */
>>>  };
>>
>>
>>
>>>
>>> -static struct attribute_group pmu_event_attr_group = {
>>> +static const struct attribute_group pmu_event_attr_group = {
>>>      .name = "events",
>>>      .attrs = NULL,        /* Filled in cci_pmu_init_attrs */
>>>  };
>>>
>>
>> I think we cannot make these const, as the attrs field gets filled in at runtime, indicated
>> by the comments next to them.
>>
>     Yes, Your are right. It's filled at run time.
>         pmu_event_attr_group.attrs = model->event_attrs;
>         pmu_format_attr_group.attrs = model->format_attrs;
>     can we make pmu_attr_group as const.? It's already initialized.
>     If you are ok with this please let me know. I can push updated patch.
>

Yes, you can do that.

Or you could go a step ahead and create per-model pmu_attr_groups[], with shared
pmu_attr_group and per-model event/format groups and tie it to the model description
and make everything const.

i.e,

static const struct attribute_group cci5xx_pmu_attr_groups[] = {
	&pmu_attr_group,
	&cci5xx_format_attr_group,
	&cci5xx_event_attr_group,
	NULL,
};

static const struct attribute_group cci400r0_pmu_attr_groups = {
	&pmu_attr_group,
	&cci400_pmu_format_attr_group,
	&cci400_pmu_event_attr_group,
	NULL,
};

static const struct attribute_group cci400r1_pmu_attr_groups = {
	&pmu_attr_group,
	&cci400_pmu_format_attr_group,
	&cci400r1_pmu_event_attr_group,
	NULL,
};

and then do :

cci_pmu->pmu = (struct pmu) {
	....
	.attr_groups = model->attr_groups,
};

Suzuki

Re: [PATCH] bus: arm-cci: constify attribute_group structures.

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4377 bytes --]

Hi Arvind,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.13-rc1 next-20170714]
[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/Arvind-Yadav/bus-arm-cci-constify-attribute_group-structures/20170704-185609
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers//bus/arm-cci.c: In function 'cci_pmu_init':
>> drivers//bus/arm-cci.c:1475:29: error: assignment of member 'attrs' in read-only object
     pmu_event_attr_group.attrs = model->event_attrs;
                                ^
   drivers//bus/arm-cci.c:1476:30: error: assignment of member 'attrs' in read-only object
     pmu_format_attr_group.attrs = model->format_attrs;
                                 ^

vim +/attrs +1475 drivers//bus/arm-cci.c

c6f85cb4 Mark Rutland      2014-06-30  1468  
c6f85cb4 Mark Rutland      2014-06-30  1469  static int cci_pmu_init(struct cci_pmu *cci_pmu, struct platform_device *pdev)
c6f85cb4 Mark Rutland      2014-06-30  1470  {
5e442eba Mark Rutland      2016-02-23  1471  	const struct cci_pmu_model *model = cci_pmu->model;
5e442eba Mark Rutland      2016-02-23  1472  	char *name = model->name;
ab5b316d Suzuki K. Poulose 2015-05-26  1473  	u32 num_cntrs;
e14cfad3 Suzuki K. Poulose 2015-05-26  1474  
5e442eba Mark Rutland      2016-02-23 @1475  	pmu_event_attr_group.attrs = model->event_attrs;
5e442eba Mark Rutland      2016-02-23  1476  	pmu_format_attr_group.attrs = model->format_attrs;
a1a076d7 Suzuki K. Poulose 2015-05-26  1477  
c6f85cb4 Mark Rutland      2014-06-30  1478  	cci_pmu->pmu = (struct pmu) {
fc17c839 Suzuki K. Poulose 2015-03-18  1479  		.name		= cci_pmu->model->name,
c6f85cb4 Mark Rutland      2014-06-30  1480  		.task_ctx_nr	= perf_invalid_context,
c6f85cb4 Mark Rutland      2014-06-30  1481  		.pmu_enable	= cci_pmu_enable,
c6f85cb4 Mark Rutland      2014-06-30  1482  		.pmu_disable	= cci_pmu_disable,
c6f85cb4 Mark Rutland      2014-06-30  1483  		.event_init	= cci_pmu_event_init,
c6f85cb4 Mark Rutland      2014-06-30  1484  		.add		= cci_pmu_add,
c6f85cb4 Mark Rutland      2014-06-30  1485  		.del		= cci_pmu_del,
c6f85cb4 Mark Rutland      2014-06-30  1486  		.start		= cci_pmu_start,
c6f85cb4 Mark Rutland      2014-06-30  1487  		.stop		= cci_pmu_stop,
c6f85cb4 Mark Rutland      2014-06-30  1488  		.read		= pmu_read,
c6f85cb4 Mark Rutland      2014-06-30  1489  		.attr_groups	= pmu_attr_groups,
b91c8f28 Punit Agrawal     2013-08-22  1490  	};
b91c8f28 Punit Agrawal     2013-08-22  1491  
b91c8f28 Punit Agrawal     2013-08-22  1492  	cci_pmu->plat_device = pdev;
ab5b316d Suzuki K. Poulose 2015-05-26  1493  	num_cntrs = pmu_get_max_counters();
ab5b316d Suzuki K. Poulose 2015-05-26  1494  	if (num_cntrs > cci_pmu->model->num_hw_cntrs) {
ab5b316d Suzuki K. Poulose 2015-05-26  1495  		dev_warn(&pdev->dev,
ab5b316d Suzuki K. Poulose 2015-05-26  1496  			"PMU implements more counters(%d) than supported by"
ab5b316d Suzuki K. Poulose 2015-05-26  1497  			" the model(%d), truncated.",
ab5b316d Suzuki K. Poulose 2015-05-26  1498  			num_cntrs, cci_pmu->model->num_hw_cntrs);
ab5b316d Suzuki K. Poulose 2015-05-26  1499  		num_cntrs = cci_pmu->model->num_hw_cntrs;
ab5b316d Suzuki K. Poulose 2015-05-26  1500  	}
ab5b316d Suzuki K. Poulose 2015-05-26  1501  	cci_pmu->num_cntrs = num_cntrs + cci_pmu->model->fixed_hw_cntrs;
b91c8f28 Punit Agrawal     2013-08-22  1502  
c6f85cb4 Mark Rutland      2014-06-30  1503  	return perf_pmu_register(&cci_pmu->pmu, name, -1);
c6f85cb4 Mark Rutland      2014-06-30  1504  }
c6f85cb4 Mark Rutland      2014-06-30  1505  

:::::: The code at line 1475 was first introduced by commit
:::::: 5e442eba342e567e2b3f1a39a24f81559f8370f7 arm-cci: simplify sysfs attr handling

:::::: TO: Mark Rutland <mark.rutland@arm.com>
:::::: CC: Will Deacon <will.deacon@arm.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 55902 bytes --]

Re: [PATCH] bus: arm-cci: constify attribute_group structures.

[arvind]

Hi,
As per discussion. please drop this patch. I will push anther update patch.

~arvind

On Sunday 16 July 2017 02:59 PM, kbuild test robot wrote:
> Hi Arvind,
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.13-rc1 next-20170714]
> [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/Arvind-Yadav/bus-arm-cci-constify-attribute_group-structures/20170704-185609
> config: arm64-allyesconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>          wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # save the attached .config to linux build tree
>          make.cross ARCH=arm64
>
> All errors (new ones prefixed by >>):
>
>     drivers//bus/arm-cci.c: In function 'cci_pmu_init':
>>> drivers//bus/arm-cci.c:1475:29: error: assignment of member 'attrs' in read-only object
>       pmu_event_attr_group.attrs = model->event_attrs;
>                                  ^
>     drivers//bus/arm-cci.c:1476:30: error: assignment of member 'attrs' in read-only object
>       pmu_format_attr_group.attrs = model->format_attrs;
>                                   ^
>
> vim +/attrs +1475 drivers//bus/arm-cci.c
>
> c6f85cb4 Mark Rutland      2014-06-30  1468
> c6f85cb4 Mark Rutland      2014-06-30  1469  static int cci_pmu_init(struct cci_pmu *cci_pmu, struct platform_device *pdev)
> c6f85cb4 Mark Rutland      2014-06-30  1470  {
> 5e442eba Mark Rutland      2016-02-23  1471  	const struct cci_pmu_model *model = cci_pmu->model;
> 5e442eba Mark Rutland      2016-02-23  1472  	char *name = model->name;
> ab5b316d Suzuki K. Poulose 2015-05-26  1473  	u32 num_cntrs;
> e14cfad3 Suzuki K. Poulose 2015-05-26  1474
> 5e442eba Mark Rutland      2016-02-23 @1475  	pmu_event_attr_group.attrs = model->event_attrs;
> 5e442eba Mark Rutland      2016-02-23  1476  	pmu_format_attr_group.attrs = model->format_attrs;
> a1a076d7 Suzuki K. Poulose 2015-05-26  1477
> c6f85cb4 Mark Rutland      2014-06-30  1478  	cci_pmu->pmu = (struct pmu) {
> fc17c839 Suzuki K. Poulose 2015-03-18  1479  		.name		= cci_pmu->model->name,
> c6f85cb4 Mark Rutland      2014-06-30  1480  		.task_ctx_nr	= perf_invalid_context,
> c6f85cb4 Mark Rutland      2014-06-30  1481  		.pmu_enable	= cci_pmu_enable,
> c6f85cb4 Mark Rutland      2014-06-30  1482  		.pmu_disable	= cci_pmu_disable,
> c6f85cb4 Mark Rutland      2014-06-30  1483  		.event_init	= cci_pmu_event_init,
> c6f85cb4 Mark Rutland      2014-06-30  1484  		.add		= cci_pmu_add,
> c6f85cb4 Mark Rutland      2014-06-30  1485  		.del		= cci_pmu_del,
> c6f85cb4 Mark Rutland      2014-06-30  1486  		.start		= cci_pmu_start,
> c6f85cb4 Mark Rutland      2014-06-30  1487  		.stop		= cci_pmu_stop,
> c6f85cb4 Mark Rutland      2014-06-30  1488  		.read		= pmu_read,
> c6f85cb4 Mark Rutland      2014-06-30  1489  		.attr_groups	= pmu_attr_groups,
> b91c8f28 Punit Agrawal     2013-08-22  1490  	};
> b91c8f28 Punit Agrawal     2013-08-22  1491
> b91c8f28 Punit Agrawal     2013-08-22  1492  	cci_pmu->plat_device = pdev;
> ab5b316d Suzuki K. Poulose 2015-05-26  1493  	num_cntrs = pmu_get_max_counters();
> ab5b316d Suzuki K. Poulose 2015-05-26  1494  	if (num_cntrs > cci_pmu->model->num_hw_cntrs) {
> ab5b316d Suzuki K. Poulose 2015-05-26  1495  		dev_warn(&pdev->dev,
> ab5b316d Suzuki K. Poulose 2015-05-26  1496  			"PMU implements more counters(%d) than supported by"
> ab5b316d Suzuki K. Poulose 2015-05-26  1497  			" the model(%d), truncated.",
> ab5b316d Suzuki K. Poulose 2015-05-26  1498  			num_cntrs, cci_pmu->model->num_hw_cntrs);
> ab5b316d Suzuki K. Poulose 2015-05-26  1499  		num_cntrs = cci_pmu->model->num_hw_cntrs;
> ab5b316d Suzuki K. Poulose 2015-05-26  1500  	}
> ab5b316d Suzuki K. Poulose 2015-05-26  1501  	cci_pmu->num_cntrs = num_cntrs + cci_pmu->model->fixed_hw_cntrs;
> b91c8f28 Punit Agrawal     2013-08-22  1502
> c6f85cb4 Mark Rutland      2014-06-30  1503  	return perf_pmu_register(&cci_pmu->pmu, name, -1);
> c6f85cb4 Mark Rutland      2014-06-30  1504  }
> c6f85cb4 Mark Rutland      2014-06-30  1505
>
> :::::: The code at line 1475 was first introduced by commit
> :::::: 5e442eba342e567e2b3f1a39a24f81559f8370f7 arm-cci: simplify sysfs attr handling
>
> :::::: TO: Mark Rutland <mark.rutland@arm.com>
> :::::: CC: Will Deacon <will.deacon@arm.com>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation