linux-kernel - Re: [PATCH] devres: Move remaining devm_alloc_percpu and devm_device_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aC8en60QI0MwnXxM@smile.fi.intel.com>
Date: Thu, 22 May 2025 15:54:55 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Pei Xiao <xiaopei01@...inos.cn>
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
	rafael@...nel.org, dakr@...nel.org, bartosz.golaszewski@...aro.org
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