Hi Mike,
Sorry for late.
[...]
@@ -1576,9 +1595,6 @@ static void tcmu_free_device(struct se_device *dev)
static void tcmu_destroy_device(struct se_device *dev)
{
struct tcmu_dev *udev = TCMU_DEV(dev);
- struct tcmu_cmd *cmd;
- bool all_expired = true;
- int i;
del_timer_sync(&udev->timeout);
@@ -1586,24 +1602,8 @@ static void tcmu_destroy_device(struct se_device *dev)
list_del(&udev->node);
mutex_unlock(&root_udev_mutex);
- vfree(udev->mb_addr);
-
- /* Upper layer should drain all requests before calling this */
- spin_lock_irq(&udev->commands_lock);
- idr_for_each_entry(&udev->commands, cmd, i) {
- if (tcmu_check_and_free_pending_cmd(cmd) != 0)
- all_expired = false;
- }
- idr_destroy(&udev->commands);
- spin_unlock_irq(&udev->commands_lock);
- WARN_ON(!all_expired);
-
- tcmu_blocks_release(udev);
-
Agree the above part needs to be delayed.
tcmu_netlink_event(udev, TCMU_CMD_REMOVED_DEVICE, 0, NULL);
- uio_unregister_device(&udev->uio_info);
-
Why does this call? After this returns userspace should have done its
close and stopped doing uio poll/read/write calls right?
Do you mean after when the uio_unregister_device() returns the
tcmu_netlink_event()
has successfully told the runner and stopped doing uio poll/read/write
calls ?
If i have gotten yours. I will say it maybe not yet.
See the following case if without :
- uio_unregister_device(&udev->uio_info);
-
There should still be a bug:
1, IOs are running
2, try to remove the target device through the configfs
3, then the TCMU will send a REMOVE netlink message to the runner in
tcmu_destroy_device().
4, since there maybe lots of IOs and the runner will wait for them being
done, which will still use the UIO device.
5, the TCMU doesn't know what the runner's status and just called
uio_unregister_device() to release the UIO device .
6, ....
And I am not very sure will the above case will cause the kernel's
another crash ?
If userspace is not doing that, why isn't it? I think that is a bug.
If we need to work around userspace then you need to drop the
uio_unregister_device call in tcmu_configure_device.
Yes, for this patch if we move it to tcmu_dev_kfre_release(), this
should be fixed too.
Thanks,
BRs
Xiubo
/* release ref from configure */
kref_put(&udev->kref, tcmu_dev_kref_release);
}
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
