LU-15808 ptlrpc: ptlrpc_set_wait() use wait_woken

ptlrpc_set_wait() using a potentially long running condition
ptlrpc_check_set() that can also block.

If it does block during ptl_send_rpc() it could potentially
trigger a warning:
   do not call blocking ops when !TASK_RUNNING

NeilBrown <neilb@suse.de> suggested to use wait_woken() instead.

Convert ptlrpc_set_wait to use wait_woken()
similar to the wait_woken() method used in ptlrpcd.

Lustre-change: https://review.whamcloud.com/56317
Lustre-commit: 930ad25733d925021fbce468568acacde219d67c

Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Signed-off-by: Shaun Tancheff <shaun.tancheff@hpe.com>
Change-Id: I544550db58fa2e89ce18a8a43a64fdea7ed57206
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-by: Neil Brown <neilb@suse.de>
Reviewed-by: Petros Koutoupis <petros.koutoupis@hpe.com>
Reviewed-by: Alexander Zarochentsev <alexander.zarochentsev@hpe.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/59108
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Zhenyu Xu <bobijam@hotmail.com>

diff --git a/libcfs/include/libcfs/linux/linux-wait.h b/libcfs/include/libcfs/linux/linux-wait.h
index b54370f..cd62fab 100644 (file)
--- a/libcfs/include/libcfs/linux/linux-wait.h
+++ b/libcfs/include/libcfs/linux/linux-wait.h
@@ -565,17 +565,6 @@ do {                                                                       \
        __ret;                                                          \
 })
 
-#define l_wait_event_abortable_timeout(wq, condition, timeout)         \
-({                                                                     \
-       sigset_t __new_blocked, __old_blocked;                          \
-       int __ret = 0;                                                  \
-       siginitsetinv(&__new_blocked, LUSTRE_FATAL_SIGS);               \
-       sigprocmask(SIG_BLOCK, &__new_blocked, &__old_blocked);         \
-       __ret = wait_event_interruptible_timeout(wq, condition, timeout);\
-       sigprocmask(SIG_SETMASK, &__old_blocked, NULL);                 \
-       __ret;                                                          \
-})
-
 #define l_wait_event_abortable_exclusive(wq, condition)                        \
 ({                                                                     \
        sigset_t __new_blocked, __old_blocked;                          \

diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c
index fff2296..a903ebd 100644 (file)
--- a/lustre/ptlrpc/client.c
+++ b/lustre/ptlrpc/client.c
@@ -1951,7 +1951,7 @@ int ptlrpc_check_set(const struct lu_env *env, struct ptlrpc_request_set *set)
                }
 
                /*
-                * ptlrpc_set_wait uses l_wait_event_abortable_timeout()
+                * ptlrpc_set_wait uses wait_woken()
                 * so it sets rq_intr regardless of individual rpc
                 * timeouts. The synchronous IO waiting path sets
                 * rq_intr irrespective of whether ptlrpcd
@@ -2466,6 +2466,7 @@ time64_t ptlrpc_set_next_timeout(struct ptlrpc_request_set *set)
 int ptlrpc_set_wait(const struct lu_env *env, struct ptlrpc_request_set *set)
 {
        struct ptlrpc_request *req;
+       sigset_t oldset, newset;
        time64_t timeout;
        int rc;
 
@@ -2482,7 +2483,14 @@ int ptlrpc_set_wait(const struct lu_env *env, struct ptlrpc_request_set *set)
                RETURN(0);
 
        do {
+               DEFINE_WAIT_FUNC(wait, woken_wake_function);
+               long remaining;
+               unsigned long allow = 0;
+               int state = TASK_IDLE;
+
+               rc = 0;
                timeout = ptlrpc_set_next_timeout(set);
+               remaining = cfs_time_seconds(timeout ? timeout : 1);
 
                /*
                 * wait until all complete, interrupted, or an in-flight
@@ -2491,71 +2499,39 @@ int ptlrpc_set_wait(const struct lu_env *env, struct ptlrpc_request_set *set)
                CDEBUG(D_RPCTRACE, "set %p going to sleep for %lld seconds\n",
                       set, timeout);
 
+               add_wait_queue(&set->set_waitq, &wait);
                if ((timeout == 0 && !signal_pending(current)) ||
                    set->set_allow_intr) {
-                       /*
-                        * No requests are in-flight (ether timed out
-                        * or delayed), so we can allow interrupts.
-                        * We still want to block for a limited time,
-                        * so we allow interrupts during the timeout.
-                        */
-                       rc = l_wait_event_abortable_timeout(
-                               set->set_waitq,
-                               ptlrpc_check_set(NULL, set),
-                               cfs_time_seconds(timeout ? timeout : 1));
-                       if (rc == 0) {
-                               rc = -ETIMEDOUT;
-                               ptlrpc_expired_set(set);
-                       } else if (rc < 0) {
-                               rc = -EINTR;
-                               ptlrpc_interrupted_set(set);
-                       } else {
-                               rc = 0;
-                       }
-               } else {
-                       /*
-                        * At least one request is in flight, so no
-                        * interrupts are allowed. Wait until all
-                        * complete, or an in-flight req times out.
-                        */
-                       rc = wait_event_idle_timeout(
-                               set->set_waitq,
-                               ptlrpc_check_set(NULL, set),
-                               cfs_time_seconds(timeout ? timeout : 1));
-                       if (rc == 0) {
-                               ptlrpc_expired_set(set);
-                               rc = -ETIMEDOUT;
-                       } else {
-                               rc = 0;
+                       state = TASK_INTERRUPTIBLE;
+                       allow = LUSTRE_FATAL_SIGS;
+               }
+               /* block until ready or timeout occurs */
+               do {
+                       if (ptlrpc_check_set(NULL, set))
+                               break;
+                       if (allow) {
+                               siginitsetinv(&newset, allow);
+                               sigprocmask(SIG_BLOCK, &newset, &oldset);
                        }
-
-                       /*
-                        * LU-769 - if we ignored the signal because
-                        * it was already pending when we started, we
-                        * need to handle it now or we risk it being
-                        * ignored forever
-                        */
-                       if (rc == -ETIMEDOUT &&
-                           signal_pending(current)) {
-                               sigset_t old, new;
-
-                               siginitset(&new, LUSTRE_FATAL_SIGS);
-                               sigprocmask(SIG_BLOCK, &new, &old);
-                               /*
-                                * In fact we only interrupt for the
-                                * "fatal" signals like SIGINT or
-                                * SIGKILL. We still ignore less
-                                * important signals since ptlrpc set
-                                * is not easily reentrant from
-                                * userspace again
-                                */
+                       remaining = wait_woken(&wait, state, remaining);
+                       if (allow) {
                                if (signal_pending(current))
-                                       ptlrpc_interrupted_set(set);
-                               sigprocmask(SIG_SETMASK, &old, NULL);
+                                       remaining = -EINTR;
+                               sigprocmask(SIG_SETMASK, &oldset, NULL);
                        }
+               } while (remaining > 0);
+               /*
+                * wait_woken* returns the result from schedule_timeout() which
+                * is always a positive number, or 0 on timeout.
+                */
+               if (remaining == 0) {
+                       rc = -ETIMEDOUT;
+                       ptlrpc_expired_set(set);
+               } else if (remaining < 0) {
+                       rc = -EINTR;
+                       ptlrpc_interrupted_set(set);
                }
-
-               LASSERT(rc == 0 || rc == -EINTR || rc == -ETIMEDOUT);
+               remove_wait_queue(&set->set_waitq, &wait);
 
                /*
                 * -EINTR => all requests have been flagged rq_intr so next