[PATCH linux dev-4.7 v2] drivers: fsi: Use kernel ida to manage device indexes

[Eddie James]

On 02/17/2017 03:01 PM, Eddie James wrote:
> From: "Edward A. James" <eajames at us.ibm.com>
>
> Fixes bad device indexes and duplicates after fsi rescan
>
> since v1:
>   * don't use the unsafe atomic counter
>
> Signed-off-by: Edward A. James <eajames at us.ibm.com>
> ---
>   drivers/fsi/fsi-core.c        | 10 ++++++++--
>   drivers/fsi/fsi-master-gpio.c |  1 +
>   drivers/fsi/fsi-scom.c        | 10 ++++++++--
>   3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index e8a3618..d63a892 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -15,6 +15,7 @@
>
>   #include <linux/device.h>
>   #include <linux/fsi.h>
> +#include <linux/idr.h>
>   #include <linux/kthread.h>
>   #include <linux/module.h>
>   #include <linux/slab.h>
> @@ -46,7 +47,7 @@ static const int engine_page_size = 0x400;
>   static struct task_struct *master_ipoll;
>   static unsigned int fsi_ipoll_period_ms = 100;
>
> -static atomic_t master_idx = ATOMIC_INIT(-1);
> +static DEFINE_IDA(master_ida);
>
>   struct fsi_slave {
>   	struct list_head	list_link;	/* Master's list of slaves */
> @@ -599,7 +600,7 @@ int fsi_master_register(struct fsi_master *master)
>   	if (!master || !master->dev)
>   		return -EINVAL;
>
> -	master->idx = atomic_inc_return(&master_idx);
> +	master->idx = ida_simple_get(&master_ida, 0, INT_MAX, GFP_KERNEL);
>   	master->slave_list = false;
>   	get_device(master->dev);
>   	fsi_master_scan(master);
> @@ -610,6 +611,11 @@ EXPORT_SYMBOL_GPL(fsi_master_register);
>
>   void fsi_master_unregister(struct fsi_master *master)
>   {
> +	if (master->idx >= 0) {
> +		ida_simple_remove(&master_ida, master->idx);
> +		master->idx = -1;
> +	}
> +
>   	device_remove_file(master->dev, &dev_attr_fsi_ipoll_period);
>   	fsi_master_unscan(master);
>   	put_device(master->dev);
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index 91bdbf2..3ed82ea 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -512,6 +512,7 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
>   	else
>   		master->gpio_mux = gpio;
>
> +	master->master.idx = -1;
>   	master->master.n_links = 1;
>   	master->master.read = fsi_master_gpio_read;
>   	master->master.write = fsi_master_gpio_write;
> diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
> index a439a5e..ff8267b 100644
> --- a/drivers/fsi/fsi-scom.c
> +++ b/drivers/fsi/fsi-scom.c
> @@ -22,6 +22,7 @@
>   #include <linux/slab.h>
>   #include <linux/miscdevice.h>
>   #include <linux/list.h>
> +#include <linux/idr.h>
>
>   #define FSI_ENGID_SCOM		0x5
>
> @@ -41,12 +42,14 @@ struct scom_device {
>   	struct fsi_device *fsi_dev;
>   	struct miscdevice mdev;
>   	char	name[32];
> +	int idx;
>   };
>
>   #define to_scom_dev(x)		container_of((x), struct scom_device, mdev)
>
>   static struct list_head scom_devices;
> -static atomic_t scom_idx = ATOMIC_INIT(0);
> +
> +static DEFINE_IDA(scom_ida);
>
>   static int put_scom(struct scom_device *scom_dev, uint64_t value,
>   			uint32_t addr)
> @@ -187,8 +190,9 @@ static int scom_probe(struct device *dev)
>   	if (!scom)
>   		return -ENOMEM;
>
> +	scom->idx = ida_simple_get(&scom_ida, 1, INT_MAX, GFP_KERNEL);
>   	snprintf(scom->name, sizeof(scom->name),
> -			"scom%d", atomic_inc_return(&scom_idx));
> +			"scom%d", scom->idx);
>   	scom->fsi_dev = fsi_dev;
>   	scom->mdev.minor = MISC_DYNAMIC_MINOR;
>   	scom->mdev.fops = &scom_fops;
> @@ -211,6 +215,8 @@ static int scom_remove(struct device *dev)
>   		}
>   	}
>
> +	ida_simple_remove(&scom_ida, scom->idx);
> +

Shouldn't access bad pointers...

nak, see v3.

>   	return 0;
>   }
>