Re: [PATCH, RESEND] Avoid that SCSI device removal through sysfs triggers a deadlock

[Tejun Heo <tj@kernel.org>]

Hello,

ISTR giving the same feedback before.

On Wed, Jul 25, 2018 at 10:38:28AM -0700, Bart Van Assche wrote:
> +struct remove_dev_work {
> +	struct callback_head	head;
> +	struct scsi_device	*sdev;
> +};
> +
> +static void delete_sdev(struct callback_head *head)
> +{
> +	struct remove_dev_work *work = container_of(head, typeof(*work), head);
> +	struct scsi_device *sdev = work->sdev;
> +
> +	scsi_remove_device(sdev);
> +	kfree(work);
> +	scsi_device_put(sdev);
> +}
> +
>  static ssize_t
>  sdev_store_delete(struct device *dev, struct device_attribute *attr,
>  		  const char *buf, size_t count)
>  {
> -	if (device_remove_file_self(dev, attr))
> -		scsi_remove_device(to_scsi_device(dev));
> -	return count;
> -};
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +	struct remove_dev_work *work;
> +	int ret;
> +
> +	ret = scsi_device_get(sdev);
> +	if (ret < 0)
> +		goto out;
> +	ret = -ENOMEM;
> +	work = kmalloc(sizeof(*work), GFP_KERNEL);
> +	if (!work)
> +		goto put;
> +	work->head.func = delete_sdev;
> +	work->sdev = sdev;
> +	ret = task_work_add(current, &work->head, false);
> +	if (ret < 0)
> +		goto free;
> +	ret = count;
> +
> +out:
> +	return ret;
> +
> +free:
> +	kfree(work);
> +
> +put:
> +	scsi_device_put(sdev);
> +	goto out;
> +}

Making removal asynchronous this way sometimes causes issues because
whether the user sees the device released or not is racy.
kernfs/sysfs have mechanisms to deal with these cases - remove_self
and kernfs_break_active_protection().  Have you looked at those?

Thanks.

-- 
tejun