LU-10467 ptlrpc: refactor waiting in ptlrpc_set_wait()

ptlrpc_set_wait can wait either with or without signals blocked.
After it has waited, it possibly checks if a signal is pending and if
so, marks the set as interrupted.

The code for the check examines lwi.lwi_allow_intr which was set
before the wait.  Converting this to use upstream wait primitives
will remove lwi, so we need another way to handle this.

The current test looks wrong.  It is
        if (rc == -ETIMEDOUT &&
            (!lwi.lwi_allow_intr || set->set_allow_intr) &&
            signal_pending(current)) {

but if lwi.lwi_allow_intr is true, then the wait will have allowed
signals and so the set will already have been interrupted if needed.
So the case where lwi.lwi_allow_intr is true and set->set_allow_intr
is also true, should be irrelevant.

i.e. the condition should just be
        if (rc == -ETIMEDOUT &&
            !lwi.lwi_allow_intr &&
            signal_pending(current)) {

which it was before
 Commit afcf3026c6ad ("LU-6684 lfsck: stop lfsck even
                       if some servers offline")

Given this, if we move the l_wait_event() into each branch of the
'if', we can then move the extra condition and
ptlrpc_interrupted_set() call into the 'else' branch - the only place
the condition would fire, and simplify the condition to

        if (rc == -ETIMEDOUT &&
            signal_pending(current)) {

This will make the two waits separate, so they can be easily
converted.

Signed-off-by: Mr NeilBrown <neilb@suse.com>
Change-Id: I1e1819c697cea47607d5fc4a018c898236b33f4b
Reviewed-on: https://review.whamcloud.com/35981
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Shaun Tancheff <shaun.tancheff@hpe.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Reviewed-by: Petros Koutoupis <pkoutoupis@cray.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c
index 85bb30d..0182bbd 100644 (file)
--- a/lustre/ptlrpc/client.c
+++ b/lustre/ptlrpc/client.c
@@ -2491,7 +2491,7 @@ int ptlrpc_set_wait(const struct lu_env *env, struct ptlrpc_request_set *set)
                       set, timeout);
 
                if ((timeout == 0 && !signal_pending(current)) ||
                       set, timeout);
 
                if ((timeout == 0 && !signal_pending(current)) ||

-                   set->set_allow_intr)
+                   set->set_allow_intr) {

                        /*
                         * No requests are in-flight (ether timed out
                         * or delayed), so we can allow interrupts.
                        /*
                         * No requests are in-flight (ether timed out
                         * or delayed), so we can allow interrupts.


@@ -2502,7 +2502,10 @@ int ptlrpc_set_wait(const struct lu_env *env, struct ptlrpc_request_set *set)
                                        cfs_time_seconds(timeout ? timeout : 1),
                                        ptlrpc_expired_set,
                                        ptlrpc_interrupted_set, set);
                                        cfs_time_seconds(timeout ? timeout : 1),
                                        ptlrpc_expired_set,
                                        ptlrpc_interrupted_set, set);

-               else
+
+                       rc = l_wait_event(set->set_waitq,
+                                         ptlrpc_check_set(NULL, set), &lwi);
+               } else {

                        /*
                         * At least one request is in flight, so no
                         * interrupts are allowed. Wait until all
                        /*
                         * At least one request is in flight, so no
                         * interrupts are allowed. Wait until all


@@ -2511,29 +2514,32 @@ int ptlrpc_set_wait(const struct lu_env *env, struct ptlrpc_request_set *set)
                        lwi = LWI_TIMEOUT(cfs_time_seconds(timeout ? timeout : 1),
                                          ptlrpc_expired_set, set);
 
                        lwi = LWI_TIMEOUT(cfs_time_seconds(timeout ? timeout : 1),
                                          ptlrpc_expired_set, set);
 

-               rc = l_wait_event(set->set_waitq,
-                                 ptlrpc_check_set(NULL, set), &lwi);
-
-               /*
-                * 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 &&
-                   (!lwi.lwi_allow_intr || set->set_allow_intr) &&
-                   signal_pending(current)) {
-                       sigset_t blocked_sigs =
-                                          cfs_block_sigsinv(LUSTRE_FATAL_SIGS);
+                       rc = l_wait_event(set->set_waitq,
+                                         ptlrpc_check_set(NULL, set), &lwi);

 
                        /*
 
                        /*

-                        * 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
+                        * 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 (signal_pending(current))
-                               ptlrpc_interrupted_set(set);
-                       cfs_restore_sigs(blocked_sigs);
+                       if (rc == -ETIMEDOUT &&
+                           signal_pending(current)) {
+                               sigset_t blocked_sigs =
+                                       cfs_block_sigsinv(LUSTRE_FATAL_SIGS);
+
+                               /*
+                                * 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
+                                */
+                               if (signal_pending(current))
+                                       ptlrpc_interrupted_set(set);
+                               cfs_restore_sigs(blocked_sigs);
+                       }

                }
 
                LASSERT(rc == 0 || rc == -EINTR || rc == -ETIMEDOUT);
                }
 
                LASSERT(rc == 0 || rc == -EINTR || rc == -ETIMEDOUT);