Hi Jean,
On 10/07/2016 05:32 AM, Jean Delvare wrote:
[ ... ]
+ name = (char *)template;
+ } else {
+ name = devm_kzalloc(dev, strlen(template) + 16, GFP_KERNEL);
+ if (!name)
+ return ERR_PTR(-ENOMEM);
+ scnprintf(name, strlen(template) + 16, template,
+ index + hwmon_attr_base(type));
+ }
+
+ hattr = devm_kzalloc(dev, sizeof(*hattr), GFP_KERNEL);
+ if (!hattr)
+ return ERR_PTR(-ENOMEM);
So basically you are doing 1 or 2 memory allocations for every
attribute? Looks quite bad from a memory fragmentation
perspective :-( Not to mention performance. Given that you have all the
data at hand before you start, can't you preallocate an array with the
right number of hattr and pick from it? That would at least solve half
of the problem.
FTR I took a quick look at the iio code and there seems to be something
like the idea above implemented in iio_device_register_sysfs(). But
attributes themselves as instantiated by iio_device_register_sysfs()
are still allocated individually. But hey I'm not familiar with the iio
code anyway, I'm sure you know it better than I do.
Something similar for string allocation may work too, although it's
somewhat more complex due to the variable length. But I think the
abituguru3 driver is doing it right, so maybe you can too.
I'll look into it.
Merging name allocation and hattr allocation turned out to be easy by adding
the name to struct hwmon_device_attribute. However, allocating an array of
struct hwmon_device_attribute for all attributes in one go is more difficult.
Main issue is that we _don't_ really know how many attributes are going to be
created; we only know the ceiling, which is calculated in __hwmon_create_attrs().
That number is currently used to allocate the array of attributes. The extra
pointers did not seem that important there (to me). However, allocating an
array of struct hwmon_device_attribute would result in memory being allocated
for non-existing attributes as well (ie those for which is_visible returns 0),
and that would at least potentially be much more. We can either accept that,
or I would have to implement a second pass over the attributes to determine
how many are actually implemented, or we could leave the per-attribute
allocation unchanged.
Thoughts ?
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
