| Message ID | 20180607145720.22590-7-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/07/2018 07:57 AM, Matthew Wilcox wrote: > From: Matthew Wilcox <mawilcox@microsoft.com> > > Need to do a bit of rearranging to make this work. > > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com> > --- > arch/x86/events/intel/uncore.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c > index 15b07379e72d..e15cfad4f89b 100644 > --- a/arch/x86/events/intel/uncore.c > +++ b/arch/x86/events/intel/uncore.c > @@ -865,8 +865,6 @@ static void uncore_types_exit(struct intel_uncore_type **types) > static int __init uncore_type_init(struct intel_uncore_type *type, bool setid) > { > struct intel_uncore_pmu *pmus; > - struct attribute_group *attr_group; > - struct attribute **attrs; > size_t size; > int i, j; > > @@ -891,21 +889,24 @@ static int __init uncore_type_init(struct intel_uncore_type *type, bool setid) > 0, type->num_counters, 0, 0); > > if (type->event_descs) { > + struct { > + struct attribute_group group; > + struct attribute *attrs[]; > + } *attr_group; > for (i = 0; type->event_descs[i].attr.attr.name; i++); What does this for loop do? Looks like nothing given the semicolon at the end. > - attr_group = kzalloc(sizeof(struct attribute *) * (i + 1) + > - sizeof(*attr_group), GFP_KERNEL); > + attr_group = kzalloc(struct_size(attr_group, attrs, i + 1), > + GFP_KERNEL); > if (!attr_group) > goto err; > > - attrs = (struct attribute **)(attr_group + 1); > - attr_group->name = "events"; > - attr_group->attrs = attrs; > + attr_group->group.name = "events"; > + attr_group->group.attrs = attr_group->attrs; > > for (j = 0; j < i; j++) > - attrs[j] = &type->event_descs[j].attr.attr; > + attr_group->attrs[j] = &type->event_descs[j].attr.attr; > > - type->events_group = attr_group; > + type->events_group = &attr_group->group; > } > > type->pmu_group = &uncore_pmu_attr_group; >
On Thu, Jun 7, 2018 at 10:30 AM Ralph Campbell <rcampbell@nvidia.com> wrote: > > > > On 06/07/2018 07:57 AM, Matthew Wilcox wrote: > > From: Matthew Wilcox <mawilcox@microsoft.com> > > > > Need to do a bit of rearranging to make this work. > > > > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com> > > --- > > arch/x86/events/intel/uncore.c | 19 ++++++++++--------- > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c > > index 15b07379e72d..e15cfad4f89b 100644 > > --- a/arch/x86/events/intel/uncore.c > > +++ b/arch/x86/events/intel/uncore.c > > @@ -865,8 +865,6 @@ static void uncore_types_exit(struct intel_uncore_type **types) > > static int __init uncore_type_init(struct intel_uncore_type *type, bool setid) > > { > > struct intel_uncore_pmu *pmus; > > - struct attribute_group *attr_group; > > - struct attribute **attrs; > > size_t size; > > int i, j; > > > > @@ -891,21 +889,24 @@ static int __init uncore_type_init(struct intel_uncore_type *type, bool setid) > > 0, type->num_counters, 0, 0); > > > > if (type->event_descs) { > > + struct { > > + struct attribute_group group; > > + struct attribute *attrs[]; > > + } *attr_group; > > for (i = 0; type->event_descs[i].attr.attr.name; i++); > > What does this for loop do? > Looks like nothing given the semicolon at the end. > Finding the first index 'i' where type->event_descs[i].attr.attr.name is NULL with the assumption that one such entry definitely exists.
Hi Matthew,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.17 next-20180607]
[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/Matthew-Wilcox/More-conversions-to-struct_size/20180608-112654
config: x86_64-randconfig-x015-201822 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
arch/x86/events/intel/uncore.c: In function 'uncore_type_init':
>> arch/x86/events/intel/uncore.c:838:24: error: implicit declaration of function 'struct_size'; did you mean 'bd_set_size'? [-Werror=implicit-function-declaration]
attr_group = kzalloc(struct_size(attr_group, attrs, i + 1),
^~~~~~~~~~~
bd_set_size
>> arch/x86/events/intel/uncore.c:838:48: error: 'attrs' undeclared (first use in this function); did you mean 'iattr'?
attr_group = kzalloc(struct_size(attr_group, attrs, i + 1),
^~~~~
iattr
arch/x86/events/intel/uncore.c:838:48: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
vim +838 arch/x86/events/intel/uncore.c
804
805 static int __init uncore_type_init(struct intel_uncore_type *type, bool setid)
806 {
807 struct intel_uncore_pmu *pmus;
808 size_t size;
809 int i, j;
810
811 pmus = kzalloc(sizeof(*pmus) * type->num_boxes, GFP_KERNEL);
812 if (!pmus)
813 return -ENOMEM;
814
815 size = max_packages * sizeof(struct intel_uncore_box *);
816
817 for (i = 0; i < type->num_boxes; i++) {
818 pmus[i].func_id = setid ? i : -1;
819 pmus[i].pmu_idx = i;
820 pmus[i].type = type;
821 pmus[i].boxes = kzalloc(size, GFP_KERNEL);
822 if (!pmus[i].boxes)
823 goto err;
824 }
825
826 type->pmus = pmus;
827 type->unconstrainted = (struct event_constraint)
828 __EVENT_CONSTRAINT(0, (1ULL << type->num_counters) - 1,
829 0, type->num_counters, 0, 0);
830
831 if (type->event_descs) {
832 struct {
833 struct attribute_group group;
834 struct attribute *attrs[];
835 } *attr_group;
836 for (i = 0; type->event_descs[i].attr.attr.name; i++);
837
> 838 attr_group = kzalloc(struct_size(attr_group, attrs, i + 1),
839 GFP_KERNEL);
840 if (!attr_group)
841 goto err;
842
843 attr_group->group.name = "events";
844 attr_group->group.attrs = attr_group->attrs;
845
846 for (j = 0; j < i; j++)
847 attr_group->attrs[j] = &type->event_descs[j].attr.attr;
848
849 type->events_group = &attr_group->group;
850 }
851
852 type->pmu_group = &uncore_pmu_attr_group;
853
854 return 0;
855
856 err:
857 for (i = 0; i < type->num_boxes; i++)
858 kfree(pmus[i].boxes);
859 kfree(pmus);
860
861 return -ENOMEM;
862 }
863
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Matthew,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.17 next-20180607]
[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/Matthew-Wilcox/More-conversions-to-struct_size/20180608-112654
config: x86_64-randconfig-x016-201822 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
arch/x86/events/intel/uncore.c: In function 'uncore_type_init':
>> arch/x86/events/intel/uncore.c:838:24: error: implicit declaration of function 'struct_size' [-Werror=implicit-function-declaration]
attr_group = kzalloc(struct_size(attr_group, attrs, i + 1),
^~~~~~~~~~~
arch/x86/events/intel/uncore.c:838:48: error: 'attrs' undeclared (first use in this function); did you mean 'iattr'?
attr_group = kzalloc(struct_size(attr_group, attrs, i + 1),
^~~~~
iattr
arch/x86/events/intel/uncore.c:838:48: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
vim +/struct_size +838 arch/x86/events/intel/uncore.c
804
805 static int __init uncore_type_init(struct intel_uncore_type *type, bool setid)
806 {
807 struct intel_uncore_pmu *pmus;
808 size_t size;
809 int i, j;
810
811 pmus = kzalloc(sizeof(*pmus) * type->num_boxes, GFP_KERNEL);
812 if (!pmus)
813 return -ENOMEM;
814
815 size = max_packages * sizeof(struct intel_uncore_box *);
816
817 for (i = 0; i < type->num_boxes; i++) {
818 pmus[i].func_id = setid ? i : -1;
819 pmus[i].pmu_idx = i;
820 pmus[i].type = type;
821 pmus[i].boxes = kzalloc(size, GFP_KERNEL);
822 if (!pmus[i].boxes)
823 goto err;
824 }
825
826 type->pmus = pmus;
827 type->unconstrainted = (struct event_constraint)
828 __EVENT_CONSTRAINT(0, (1ULL << type->num_counters) - 1,
829 0, type->num_counters, 0, 0);
830
831 if (type->event_descs) {
832 struct {
833 struct attribute_group group;
834 struct attribute *attrs[];
835 } *attr_group;
836 for (i = 0; type->event_descs[i].attr.attr.name; i++);
837
> 838 attr_group = kzalloc(struct_size(attr_group, attrs, i + 1),
839 GFP_KERNEL);
840 if (!attr_group)
841 goto err;
842
843 attr_group->group.name = "events";
844 attr_group->group.attrs = attr_group->attrs;
845
846 for (j = 0; j < i; j++)
847 attr_group->attrs[j] = &type->event_descs[j].attr.attr;
848
849 type->events_group = &attr_group->group;
850 }
851
852 type->pmu_group = &uncore_pmu_attr_group;
853
854 return 0;
855
856 err:
857 for (i = 0; i < type->num_boxes; i++)
858 kfree(pmus[i].boxes);
859 kfree(pmus);
860
861 return -ENOMEM;
862 }
863
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c index 15b07379e72d..e15cfad4f89b 100644 --- a/arch/x86/events/intel/uncore.c +++ b/arch/x86/events/intel/uncore.c @@ -865,8 +865,6 @@ static void uncore_types_exit(struct intel_uncore_type **types) static int __init uncore_type_init(struct intel_uncore_type *type, bool setid) { struct intel_uncore_pmu *pmus; - struct attribute_group *attr_group; - struct attribute **attrs; size_t size; int i, j; @@ -891,21 +889,24 @@ static int __init uncore_type_init(struct intel_uncore_type *type, bool setid) 0, type->num_counters, 0, 0); if (type->event_descs) { + struct { + struct attribute_group group; + struct attribute *attrs[]; + } *attr_group; for (i = 0; type->event_descs[i].attr.attr.name; i++); - attr_group = kzalloc(sizeof(struct attribute *) * (i + 1) + - sizeof(*attr_group), GFP_KERNEL); + attr_group = kzalloc(struct_size(attr_group, attrs, i + 1), + GFP_KERNEL); if (!attr_group) goto err; - attrs = (struct attribute **)(attr_group + 1); - attr_group->name = "events"; - attr_group->attrs = attrs; + attr_group->group.name = "events"; + attr_group->group.attrs = attr_group->attrs; for (j = 0; j < i; j++) - attrs[j] = &type->event_descs[j].attr.attr; + attr_group->attrs[j] = &type->event_descs[j].attr.attr; - type->events_group = attr_group; + type->events_group = &attr_group->group; } type->pmu_group = &uncore_pmu_attr_group;