Whamcloud - gitweb
LU-16633 obdclass: fix rpc slot leakage 61/50261/12
authorAlex Zhuravlev <bzzz@whamcloud.com>
Fri, 10 Mar 2023 17:47:05 +0000 (20:47 +0300)
committerOleg Drokin <green@whamcloud.com>
Tue, 28 Mar 2023 22:18:21 +0000 (22:18 +0000)
obd_get_mod_rpc_slot() can race with obd_put_mod_rpc_slot():
finishing wait_woken() resets WQ_FLAG_WOKEN (which is set
when the corresponding thread gets a slot incrementing
cl_mod_rpcs_in_flight. then another thread execting
__wake_up_locked_key() may find that wq_entry again and call
claim_mod_rpc_function() one more time again incrementing
cl_mod_rpc_in_flight. thus it's incremented twice for a
single obd_get_mod_rpc_slot().

 #1: obd_get_mod_rpc_slot() #2: obd_put_mod_rpc_slot()
flags &= ~WQ_FLAG_WOKEN
list_add()
wait_woken()
schedule claim_mod_rpc_function()
cl_mod_rpcs_in_flight++
wake_up()

flags &= ~WQ_FLAG_WOKEN

#3: obd_put_mod_rpc_slot()
claim_mod_rpc_function()
cl_mod_rpcs_in_flight++
wake_up()
list_del()

the patch introduces a replacement for WQ_FLAG_WOKEN which is never
reset once set.

Fixes: 5243630b09 ("LU-15947 obdclass: improve precision of wakeups for mod_rpcs")
Signed-off-by: Alex Zhuravlev <bzzz@whamcloud.com>
Change-Id: I29371c8c85414413c5a8e41dec3632f64ad127bb
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/50261
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Lai Siyao <lai.siyao@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/mdc/mdc_request.c
lustre/obdclass/genops.c

index 0fcdb49..32a54c9 100644 (file)
@@ -2959,6 +2959,8 @@ static int mdc_precleanup(struct obd_device *obd)
 
 static int mdc_cleanup(struct obd_device *obd)
 {
+       struct client_obd *cli = &obd->u.cli;
+       LASSERT(cli->cl_mod_rpcs_in_flight == 0);
        return osc_cleanup_common(obd);
 }
 
index 1dcd579..ae0bca5 100644 (file)
@@ -2247,6 +2247,7 @@ EXPORT_SYMBOL(obd_mod_rpc_stats_seq_show);
 struct mod_waiter {
        struct client_obd *cli;
        bool close_req;
+       bool woken;
        wait_queue_entry_t wqe;
 };
 static int claim_mod_rpc_function(wait_queue_entry_t *wq_entry,
@@ -2259,10 +2260,9 @@ static int claim_mod_rpc_function(wait_queue_entry_t *wq_entry,
        int ret;
 
        /* As woken_wake_function() doesn't remove us from the wait_queue,
-        * we could get called twice for the same thread - take care.
+        * we use own flag to ensure we're called just once.
         */
-       if (wq_entry->flags & WQ_FLAG_WOKEN)
-               /* Already woke this thread, don't try again */
+       if (w->woken)
                return 0;
 
        /* A slot is available if
@@ -2276,6 +2276,7 @@ static int claim_mod_rpc_function(wait_queue_entry_t *wq_entry,
                if (w->close_req)
                        cli->cl_close_rpcs_in_flight++;
                ret = woken_wake_function(wq_entry, mode, flags, key);
+               w->woken = true;
        } else if (cli->cl_close_rpcs_in_flight)
                /* No other waiter could be woken */
                ret = -1;
@@ -2303,6 +2304,7 @@ __u16 obd_get_mod_rpc_slot(struct client_obd *cli, __u32 opc)
        struct mod_waiter wait = {
                .cli = cli,
                .close_req = (opc == MDS_CLOSE),
+               .woken = false,
        };
        __u16                   i, max;
 
@@ -2316,7 +2318,8 @@ __u16 obd_get_mod_rpc_slot(struct client_obd *cli, __u32 opc)
         * and there will be no need to wait.
         */
        wake_up_locked(&cli->cl_mod_rpcs_waitq);
-       if (!(wait.wqe.flags & WQ_FLAG_WOKEN)) {
+       /* XXX: handle spurious wakeups (from unknown yet source */
+       while (wait.woken == false) {
                spin_unlock_irq(&cli->cl_mod_rpcs_waitq.lock);
                wait_woken(&wait.wqe, TASK_UNINTERRUPTIBLE,
                           MAX_SCHEDULE_TIMEOUT);