On 2021/08/28 10:10, Tetsuo Handa wrote:
> If we don't ignore forced module unload, we could update my patch to keep only
> mutex_destroy() and kfree() deferred by a refcount, for only lo->lo_state,
> lo->lo_refcnt and lo->lo_encryption would be accessed under lo->lo_mutex
> serialization. There is no need to defer "del_gendisk() + idr_remove()"
> sequence for concurrent callers.
>
OK, here is a delta patch to make it no longer best effort.
We can consider removal of cryptoloop module after this patch,
starting from a printk() for deprecated message.
drivers/block/loop.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2113,7 +2113,11 @@ int loop_register_transfer(struct loop_func_table *funcs)
return 0;
}
-static void loop_remove(struct loop_device *lo);
+static void loop_destroy(struct loop_device *lo)
+{
+ mutex_destroy(&lo->lo_mutex);
+ kfree(lo);
+}
int loop_unregister_transfer(int number)
{
@@ -2137,7 +2141,7 @@ int loop_unregister_transfer(int number)
loop_release_xfer(lo);
mutex_unlock(&lo->lo_mutex);
if (refcount_dec_and_test(&lo->idr_visible))
- loop_remove(lo);
+ loop_destroy(lo);
spin_lock(&loop_idr_spinlock);
}
spin_unlock(&loop_idr_spinlock);
@@ -2426,9 +2430,6 @@ static void loop_remove(struct loop_device *lo)
spin_lock(&loop_idr_spinlock);
idr_remove(&loop_index_idr, lo->lo_number);
spin_unlock(&loop_idr_spinlock);
- /* There is no route which can find this loop device. */
- mutex_destroy(&lo->lo_mutex);
- kfree(lo);
}
static void loop_probe(dev_t dev)
@@ -2452,7 +2453,7 @@ static int loop_control_remove(int idx)
/*
* Identify the loop device to remove. Skip the device if it is owned by
- * loop_remove()/loop_add() where it is not safe to access lo_mutex.
+ * loop_add() where it is not safe to access lo_mutex.
*/
spin_lock(&loop_idr_spinlock);
lo = idr_find(&loop_index_idr, idx);
@@ -2479,19 +2480,11 @@ static int loop_control_remove(int idx)
mutex_unlock(&lo->lo_mutex);
/* Hide this loop device. */
refcount_dec(&lo->idr_visible);
- /*
- * Try to wait for concurrent callers (they should complete shortly due to
- * lo->lo_state == Lo_deleting) operating on this loop device, in order to
- * help that subsequent loop_add() will not to fail with -EEXIST.
- * Note that this is best effort.
- */
- for (ret = 0; refcount_read(&lo->idr_visible) != 1 && ret < HZ; ret++)
- schedule_timeout_killable(1);
- ret = 0;
+ /* Remove this loop device, but wait concurrent callers before destroy. */
+ loop_remove(lo);
out:
- /* Remove this loop device. */
if (refcount_dec_and_test(&lo->idr_visible))
- loop_remove(lo);
+ loop_destroy(lo);
return ret;
}
@@ -2623,8 +2616,10 @@ static void __exit loop_exit(void)
* There is no need to use loop_idr_spinlock here, for nobody else can
* access loop_index_idr when this module is unloading.
*/
- idr_for_each_entry(&loop_index_idr, lo, id)
+ idr_for_each_entry(&loop_index_idr, lo, id) {
loop_remove(lo);
+ loop_destroy(lo);
+ }
idr_destroy(&loop_index_idr);
}
--
2.25.1
