Re: [PATCH 3/3] platform/x86: dell-smbios-wmi: Replace global list with single item

Ilpo Järvinen <[email protected]>
Newsgroups org.kernel.vger.platform-driver-x86,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Thu, 16 Jul 2026, Armin Wolf wrote:

> There can only exist a single instance of the dell-smbios-wmi driver
> at the same time because of naming conflicts with the character
> device ("wmi/dell-smbios"). Having a global list for all instances
> thus makes no sense.
> 
> Replace the global list with a single item used by the character
> device. This simplifies the driver and allows us to mark it as
> being multi-instance safe.
> 
> Signed-off-by: Armin Wolf <[email protected]>
> ---
>  drivers/platform/x86/dell/dell-smbios-wmi.c | 67 ++++++++++-----------
>  1 file changed, 31 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
> index 552b80429688..f9443ddfff55 100644
> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
> @@ -10,7 +10,6 @@
>  #include <linux/device.h>
>  #include <linux/dmi.h>
>  #include <linux/fs.h>
> -#include <linux/list.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> @@ -21,7 +20,6 @@
>  #include "dell-smbios.h"
>  #include "dell-wmi-descriptor.h"
>  
> -static DECLARE_RWSEM(list_lock);
>  static int wmi_supported;
>  
>  struct misc_bios_flags_structure {
> @@ -35,20 +33,14 @@ struct misc_bios_flags_structure {
>  struct wmi_smbios_priv {
>  	struct mutex call_lock; /* Protects the content of the SMBIOS buffer */
>  	struct dell_wmi_smbios_buffer *buf;
> -	struct list_head list;
>  	struct wmi_device *wdev;
>  	struct device *child;
>  	u64 req_buf_size;
>  	struct miscdevice char_dev;
>  };
> -static LIST_HEAD(wmi_list);

Okay, nevermind the comments on patch 1.

--
 i.

>  
> -static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
> -{
> -	return list_first_entry_or_null(&wmi_list,
> -					struct wmi_smbios_priv,
> -					list);
> -}
> +static DECLARE_RWSEM(chardev_lock);	/* Protects chardev_priv */
> +static struct wmi_smbios_priv *chardev_priv;
>  
>  static int run_smbios_call(struct wmi_device *wdev)
>  {
> @@ -105,16 +97,13 @@ static int dell_smbios_wmi_call(struct device *dev, struct calling_interface_buf
>  static ssize_t dell_smbios_wmi_read(struct file *filp, char __user *buffer, size_t length,
>  				    loff_t *offset)
>  {
> -	struct wmi_smbios_priv *priv;
> -
> -	guard(rwsem_read)(&list_lock);
> +	guard(rwsem_read)(&chardev_lock);
>  
> -	priv = get_first_smbios_priv();
> -	if (!priv)
> +	if (!chardev_priv)
>  		return -ENODEV;
>  
> -	return simple_read_from_buffer(buffer, length, offset, &priv->req_buf_size,
> -				       sizeof(priv->req_buf_size));
> +	return simple_read_from_buffer(buffer, length, offset, &chardev_priv->req_buf_size,
> +				       sizeof(chardev_priv->req_buf_size));
>  }
>  
>  static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
> @@ -158,20 +147,18 @@ static long dell_smbios_wmi_do_ioctl(struct wmi_smbios_priv *priv,
>  static long dell_smbios_wmi_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
>  	struct dell_wmi_smbios_buffer __user *input = (struct dell_wmi_smbios_buffer __user *)arg;
> -	struct wmi_smbios_priv *priv;
>  
>  	if (cmd != DELL_WMI_SMBIOS_CMD)
>  		return -ENOIOCTLCMD;
>  
> -	guard(rwsem_read)(&list_lock);
> +	guard(rwsem_read)(&chardev_lock);
>  
> -	priv = get_first_smbios_priv();
> -	if (!priv)
> +	if (!chardev_priv)
>  		return -ENODEV;
>  
> -	guard(mutex)(&priv->call_lock);
> +	guard(mutex)(&chardev_priv->call_lock);
>  
> -	return dell_smbios_wmi_do_ioctl(priv, input);
> +	return dell_smbios_wmi_do_ioctl(chardev_priv, input);
>  }
>  
>  static const struct file_operations dell_smbios_wmi_fops = {
> @@ -188,10 +175,29 @@ static void dell_smbios_wmi_unregister_chardev(void *data)
>  	misc_deregister(char_dev);
>  }
>  
> +static void dell_smbios_wmi_clear_chardev(void *data)
> +{
> +	guard(rwsem_write)(&chardev_lock);
> +
> +	chardev_priv = NULL;
> +}
> +
>  static int dell_smbios_wmi_register_chardev(struct wmi_smbios_priv *priv)
>  {
>  	int ret;
>  
> +	scoped_guard(rwsem_write, &chardev_lock) {
> +		/* We can only have a single chardev at a time */
> +		if (chardev_priv)
> +			return -EBUSY;
> +
> +		chardev_priv = priv;
> +	}
> +
> +	ret = devm_add_action_or_reset(&priv->wdev->dev, dell_smbios_wmi_clear_chardev, NULL);
> +	if (ret < 0)
> +		return ret;
> +
>  	priv->char_dev.minor = MISC_DYNAMIC_MINOR;
>  	priv->char_dev.name = "wmi/dell-smbios";
>  	priv->char_dev.fops = &dell_smbios_wmi_fops;
> @@ -255,24 +261,12 @@ static int dell_smbios_wmi_probe(struct wmi_device *wdev, const void *context)
>  	if (ret)
>  		return ret;
>  
> -	ret = dell_smbios_register_device(&wdev->dev, 1, &dell_smbios_wmi_call);
> -	if (ret)
> -		return ret;
> -
> -	guard(rwsem_write)(&list_lock);
> -	list_add_tail(&priv->list, &wmi_list);
> -
> -	return 0;
> +	return dell_smbios_register_device(&wdev->dev, 1, &dell_smbios_wmi_call);
>  }
>  
>  static void dell_smbios_wmi_remove(struct wmi_device *wdev)
>  {
> -	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
> -
>  	dell_smbios_unregister_device(&wdev->dev);
> -
> -	guard(rwsem_write)(&list_lock);
> -	list_del(&priv->list);
>  }
>  
>  static const struct wmi_device_id dell_smbios_wmi_id_table[] = {
> @@ -310,6 +304,7 @@ static struct wmi_driver dell_smbios_wmi_driver = {
>  	.probe = dell_smbios_wmi_probe,
>  	.remove = dell_smbios_wmi_remove,
>  	.id_table = dell_smbios_wmi_id_table,
> +	.no_singleton = true,
>  };
>  
>  int init_dell_smbios_wmi(void)
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.