Re: [PATCH] devres: Move remaining devm_alloc_percpu and devm_device_add_group to devres.h

[Pei Xiao <xiaopei01@kylinos.cn>]

在 2025/5/22 20:54, Andy Shevchenko 写道:
> 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?

I add these for build error:

In file included from drivers/gpio/gpiolib-devres.c:9:
./include/linux/device/devres.h:189:15: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘*’ token
  189 | void __percpu *__devm_alloc_percpu(struct device *dev, size_t size,
      |               ^
./include/linux/device/devres.h:191:57: error: expected ‘;’, ‘,’ or ‘)’ before ‘*’ token
  191 | void devm_free_percpu(struct device *dev, void __percpu *pdata);
      |                                                         ^
./include/linux/device/devres.h:194:25: warning: ‘struct attribute_group’ declared inside parameter list will not be visible outside of this definition or declaration
  194 |            const struct attribute_group *grp);
      |                         ^~~~~~~~~~~~~~~

> ...
>
>> +/**
>> + * 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).
ok,get it
>> + * 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.
ok
>> +void __percpu *__devm_alloc_percpu(struct device *dev, size_t size,
>> +				   size_t align);
> Ditto.
ok.
>> +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.
>