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 <[email protected]>
---
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
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 <[email protected]>
>From a quick scan, this looks sane to me:
Acked-by: Mark Rutland <[email protected]>
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
>
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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
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 <[email protected]>
>>> ---
>>> 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
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 <[email protected]>
:::::: CC: Will Deacon <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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 <[email protected]>
> :::::: CC: Will Deacon <[email protected]>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation