On 2021/08/25 22:15, Christoph Hellwig wrote:
> On Sun, Aug 22, 2021 at 12:46:29AM +0900, Tetsuo Handa wrote:
>> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
>> ---
>> Only compile tested. This patch is a hint for Christoph Hellwig
>> who needs to know what the global mutex was serializing...
>
> Thanks a lot for your feedback. Most of this looks good, except
> for a few bits that could be done cleaner, and the fact that
> we really should be using a global waitqueue instead of a completion.
>
> Based on the recent syzbot report I'v reshuffled this and will send
> a series in a bit.
>
Thank you for responding.
I guess you might want below diff, for reinit_completion() immediately after
complete_all() likely resets completion count before all threads sleeping at
wait_for_completion_timeout() check the completion count.
Use of simple sequence count will be robuster.
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -51,7 +51,8 @@ static DEFINE_IDR(nbd_index_idr);
static DEFINE_MUTEX(nbd_index_mutex);
static struct workqueue_struct *nbd_del_wq;
static int nbd_total_devices = 0;
-static DECLARE_COMPLETION(nbd_destroy_complete);
+static atomic_t nbd_destroy_sequence = ATOMIC_INIT(0);
+static DECLARE_WAIT_QUEUE_HEAD(nbd_destroy_wait);
struct nbd_sock {
struct socket *sock;
@@ -244,8 +245,8 @@ static const struct device_attribute backend_attr = {
static void nbd_notify_destroy_completion(struct nbd_device *nbd)
{
if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags)) {
- complete_all(&nbd_destroy_complete);
- reinit_completion(&nbd_destroy_complete);
+ atomic_inc(&nbd_destroy_sequence);
+ wake_up_all(&nbd_destroy_wait);
}
}
@@ -1871,10 +1872,12 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info)
if (nbd) {
if (test_bit(NBD_DESTROY_ON_DISCONNECT, &nbd->flags) &&
test_bit(NBD_DISCONNECT_REQUESTED, &nbd->flags)) {
+ const int seq = atomic_read(&nbd_destroy_sequence);
+
mutex_unlock(&nbd_index_mutex);
/* wait until the nbd device is completely destroyed */
- wait_for_completion_timeout(&nbd_destroy_complete, HZ / 10);
+ wait_event(nbd_destroy_wait, atomic_read(&nbd_destroy_sequence) != seq);
goto again;
}
