lkml.org 
[lkml]   [2025]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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



\
 
 \ /
  Last update: 2025-05-22 14:55    [from the cache]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and my Meterkast|Read the blog