Re: [PATCHv2 2/3] nvme: Add device_update_groups()

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Feb 03, 2022 at 04:17:47PM -0500, Martin Belanger wrote:
> From: Martin Belanger <martin.belanger@dell.com>
> 
> This is used to update the sysfs groups after device objects are fully
> initialized.

Ick, no, why?

What needs this and how is userspace going to handle groups changing
after the device is already announced to userspace?


> 
> Signed-off-by: Martin Belanger <martin.belanger@dell.com>
> ---
>  drivers/base/core.c    | 28 ++++++++++++++++++++++++++++
>  include/linux/device.h |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 3d6430eb0c6a..0b5f6d5d2459 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4473,6 +4473,34 @@ int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
>  }
>  EXPORT_SYMBOL_GPL(device_change_owner);
>  
> +/**
> + * device_update_groups - update the sysfs groups
> + * @dev: device.
> + *
> + * Returns 0 on success or error code on failure.
> + */
> +int device_update_groups(struct device *dev)
> +{
> +	struct class *class = dev->class;
> +	const struct device_type *type = dev->type;
> +	int error;
> +
> +	if (class) {
> +		error = sysfs_update_groups(&dev->kobj, class->dev_groups);

So you added new groups based on the class for the device after it was
registered?

That feels really wrong.


> +		if (error)
> +			return error;
> +	}
> +
> +	if (type) {
> +		error = sysfs_update_groups(&dev->kobj, type->groups);

How can the type groups have changed for the device after it was
registered?

What problem are you trying to solve here that requires this?  How are
you changing the groups for a class or type after a device is
registered?

confused,

greg k-h