From 609246d13db6de87d1cf32b34607346ff56dd30d Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Fri, 10 Jan 2020 09:01:32 -0500 Subject: [PATCH] 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 Change-Id: I1e1819c697cea47607d5fc4a018c898236b33f4b Reviewed-on: https://review.whamcloud.com/35981 Tested-by: jenkins Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Shaun Tancheff Reviewed-by: Mike Pershin Reviewed-by: Petros Koutoupis Reviewed-by: Oleg Drokin --- lustre/ptlrpc/client.c | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c index 85bb30d..0182bbd 100644 --- 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->set_allow_intr) + set->set_allow_intr) { /* * 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); - 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 @@ -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); - 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); -- 1.8.3.1