Messages in this thread |  | | Date | Thu, 22 May 2025 15:54:55 +0300 | | From | Andy Shevchenko <> | | Subject | Re: [PATCH] devres: Move remaining devm_alloc_percpu and devm_device_add_group to devres.h |
| |
On Thu, May 22, 2025 at 05:01:26PM +0800, Pei Xiao wrote: > Since commit f5e5631df596("devres: Move devm_*_action*() APIs to > devres.h"), But devm_alloc_percpu() and devm_device_add_group didn't be > moved. > > so move it.The changes improve header organization by keeping all > resource-managed device APIs in the dedicated devres.h header, > reducing cross-header dependencies and making the interfaces > easier to locate.
Thanks for the patch, my comments below.
...
> -/** > - * devm_alloc_percpu - Resource-managed alloc_percpu > - * @dev: Device to allocate per-cpu memory for > - * @type: Type to allocate per-cpu memory for > - * > - * Managed alloc_percpu. Per-cpu memory allocated with this function is > - * automatically freed on driver detach. > - * > - * RETURNS: > - * Pointer to allocated memory on success, NULL on failure. > - */ > -#define devm_alloc_percpu(dev, type) \ > - ((typeof(type) __percpu *)__devm_alloc_percpu((dev), sizeof(type), \ > - __alignof__(type))) > - > -void __percpu *__devm_alloc_percpu(struct device *dev, size_t size, > - size_t align); > -void devm_free_percpu(struct device *dev, void __percpu *pdata);
Don't you need to cleanup the header inclusions as well?
...
> -int __must_check devm_device_add_group(struct device *dev, > - const struct attribute_group *grp);
I'm not sure about this. The percpu seems standalone piece, but this has relations with the other group related definitions just above.
...
> +#include <linux/sysfs.h>
> +#include <asm/percpu.h>
What for are these new inclusions, please?
...
> +/** > + * devm_alloc_percpu - Resource-managed alloc_percpu > + * @dev: Device to allocate per-cpu memory for > + * @type: Type to allocate per-cpu memory for > + * > + * Managed alloc_percpu. Per-cpu memory allocated with this function is > + * automatically freed on driver detach. > + * > + * RETURNS:
Please, check that all of the kernel-doc are in align (using same style).
> + * Pointer to allocated memory on success, NULL on failure. > + */ > +#define devm_alloc_percpu(dev, type) \ > + ((typeof(type) __percpu *)__devm_alloc_percpu((dev), sizeof(type), \ > + __alignof__(type)))
Just make it one line. it will be less than 100.
> +void __percpu *__devm_alloc_percpu(struct device *dev, size_t size, > + size_t align);
Ditto.
> +void devm_free_percpu(struct device *dev, void __percpu *pdata);
Please, take your time to understand what is behind the __percpu and why the asm/percpu.h is redundant here.
-- With Best Regards, Andy Shevchenko
|  |