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
