From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: ACJfBovNARAvFkuIAo8IIqfYPjZhFGYixDhG3mV8Xy1lg42I9wG1Ec7ot+/meWNaSXDBeS26aP/6 ARC-Seal: i=1; a=rsa-sha256; t=1516203370; cv=none; d=google.com; s=arc-20160816; b=PkNw+jvqiT3YmnBqPQN4gEla/vpWTLJBXMJtXiJHuNEo0/jFVH8GSjazdbfVf7T/mF lcUnN7Lr95PNsOrKGwrwaX3Q5nydwlw/jjIoNr+WB6qdAXUricDcJXUW5jSG0v3JoYPk VxYhPHZxBLT1I/W99tkOEbQY+QudNGrSH1Qd27gWpTVdTx1+6UMmVeknc4AQsZx/VF0h XPCNI+Xa0axF3qa0RLMDhhs1GVKlmdCL2M9iw0Zzn3lpoQ0WPwRr8msI5Ray1rOERm6f BAiYvrYBLmte4rQhaY/CW/RJkJXnHM+vDLfHcsI0Q8TjlI110U4k1XfJqjct4wEIWaP7 lF0Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:dkim-signature:arc-authentication-results; bh=szZ4hv2yAQtwj5KF7deGelwSn7MXaO770vfoD6C7KLY=; b=o33/9K8XeacrNsGSRHAb5/MV4N83aU47pqe0mJZBfSgaOYa9x8sk0pfmeOcR/UVAeN vZk4oUbpavLSFJd0PW6jWEJG9KRRFfoPLhpC/n70Tzigja6PCI5hbdcwdcATg0vfMJoj HytPmAHMbbQZxKUsJI8/HbySvaGFFwBDiUmisg6bsZfno2rCNNPPkVH0las+Kfndt96d Wm5BAKsiqgqRgXZItzMQEranK0Sda5x9yUqdmD+qdIy/svWqPoz5uMShd2H0P8NlgZCf C75RNnhmCppkHLXZu4vl1/rJPMne5Nix60u/yAH/K9XmJJ3wchhoJw2EWDia8ENiW+3t X+LA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@infradead.org header.s=casper.20170209 header.b=kWSDFy4P; spf=pass (google.com: best guess record for domain of jsimmons@infradead.org designates 2001:8b0:10b:1236::1 as permitted sender) smtp.mailfrom=jsimmons@infradead.org Authentication-Results: mx.google.com; dkim=pass header.i=@infradead.org header.s=casper.20170209 header.b=kWSDFy4P; spf=pass (google.com: best guess record for domain of jsimmons@infradead.org designates 2001:8b0:10b:1236::1 as permitted sender) smtp.mailfrom=jsimmons@infradead.org Date: Wed, 17 Jan 2018 15:36:05 +0000 (GMT) From: James Simmons To: NeilBrown cc: Oleg Drokin , Andreas Dilger , Greg Kroah-Hartman , lkml , lustre Subject: Re: [PATCH 17/19] staging: lustre: remove l_wait_event from ptlrpc_set_wait In-Reply-To: <151538209394.23920.3578408043898411296.stgit@noble> Message-ID: References: <151538168618.23920.8261096424342988792.stgit@noble> <151538209394.23920.3578408043898411296.stgit@noble> User-Agent: Alpine 2.21 (LFD 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180117_153605_966861_2D0C7A83 X-CRM114-Status: GOOD ( 33.76 ) X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.4.1 on casper.infradead.org summary: Content analysis details: (-1.9 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 NO_RELAYS Informational: message was not relayed via SMTP -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1588993485756005367?= X-GMAIL-MSGID: =?utf-8?q?1589854465243217784?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: > This is the last remaining use of l_wait_event(). > It is the only use of LWI_TIMEOUT_INTR_ALL() which > has a meaning that timeouts can be interrupted. > Only interrupts by "fatal" signals are allowed, so > introduce l_wait_event_abortable_timeout() to > support this. Reviewed-by: James Simmons > Signed-off-by: NeilBrown > --- > drivers/staging/lustre/lustre/include/lustre_lib.h | 14 +++ > drivers/staging/lustre/lustre/ptlrpc/client.c | 84 ++++++++------------ > 2 files changed, 49 insertions(+), 49 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/include/lustre_lib.h b/drivers/staging/lustre/lustre/include/lustre_lib.h > index 1939e959b92a..ccc1a329e42b 100644 > --- a/drivers/staging/lustre/lustre/include/lustre_lib.h > +++ b/drivers/staging/lustre/lustre/include/lustre_lib.h > @@ -196,6 +196,10 @@ struct l_wait_info { > #define LUSTRE_FATAL_SIGS (sigmask(SIGKILL) | sigmask(SIGINT) | \ > sigmask(SIGTERM) | sigmask(SIGQUIT) | \ > sigmask(SIGALRM)) > +static inline int l_fatal_signal_pending(struct task_struct *p) > +{ > + return signal_pending(p) && sigtestsetmask(&p->pending.signal, LUSTRE_FATAL_SIGS); > +} > > /** > * wait_queue_entry_t of Linux (version < 2.6.34) is a FIFO list for exclusively > @@ -347,6 +351,16 @@ do { \ > __ret; \ > }) > > +#define l_wait_event_abortable_timeout(wq, condition, timeout) \ > +({ \ > + sigset_t __blocked; \ > + int __ret = 0; \ > + __blocked = cfs_block_sigsinv(LUSTRE_FATAL_SIGS); \ > + __ret = wait_event_interruptible_timeout(wq, condition, timeout);\ > + cfs_restore_sigs(__blocked); \ > + __ret; \ > +}) > + > #define l_wait_event_abortable_exclusive(wq, condition) \ > ({ \ > sigset_t __blocked; \ > diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c > index ffdd3ffd62c6..3d689d6100bc 100644 > --- a/drivers/staging/lustre/lustre/ptlrpc/client.c > +++ b/drivers/staging/lustre/lustre/ptlrpc/client.c > @@ -1774,7 +1774,7 @@ int ptlrpc_check_set(const struct lu_env *env, struct ptlrpc_request_set *set) > } > > /* > - * ptlrpc_set_wait->l_wait_event sets lwi_allow_intr > + * ptlrpc_set_wait allow signal to abort the timeout > * so it sets rq_intr regardless of individual rpc > * timeouts. The synchronous IO waiting path sets > * rq_intr irrespective of whether ptlrpcd > @@ -2122,8 +2122,7 @@ int ptlrpc_expire_one_request(struct ptlrpc_request *req, int async_unlink) > > /** > * Time out all uncompleted requests in request set pointed by \a data > - * Callback used when waiting on sets with l_wait_event. > - * Always returns 1. > + * Called when wait_event_idle_timeout times out. > */ > void ptlrpc_expired_set(struct ptlrpc_request_set *set) > { > @@ -2156,18 +2155,6 @@ void ptlrpc_expired_set(struct ptlrpc_request_set *set) > ptlrpc_expire_one_request(req, 1); > } > } > -static int ptlrpc_expired_set_void(void *data) > -{ > - struct ptlrpc_request_set *set = data; > - > - ptlrpc_expired_set(set); > - /* > - * When waiting for a whole set, we always break out of the > - * sleep so we can recalculate the timeout, or enable interrupts > - * if everyone's timed out. > - */ > - return 1; > -} > > /** > * Sets rq_intr flag in \a req under spinlock. > @@ -2182,11 +2169,10 @@ EXPORT_SYMBOL(ptlrpc_mark_interrupted); > > /** > * Interrupts (sets interrupted flag) all uncompleted requests in > - * a set \a data. Callback for l_wait_event for interruptible waits. > + * a set \a data. Called when l_wait_event_abortable_timeout receives signal. > */ > -static void ptlrpc_interrupted_set(void *data) > +static void ptlrpc_interrupted_set(struct ptlrpc_request_set *set) > { > - struct ptlrpc_request_set *set = data; > struct list_head *tmp; > > CDEBUG(D_RPCTRACE, "INTERRUPTED SET %p\n", set); > @@ -2256,7 +2242,6 @@ int ptlrpc_set_wait(struct ptlrpc_request_set *set) > { > struct list_head *tmp; > struct ptlrpc_request *req; > - struct l_wait_info lwi; > int rc, timeout; > > if (set->set_producer) > @@ -2282,46 +2267,47 @@ int ptlrpc_set_wait(struct ptlrpc_request_set *set) > CDEBUG(D_RPCTRACE, "set %p going to sleep for %d seconds\n", > set, timeout); > > - if (timeout == 0 && !signal_pending(current)) > + if (timeout == 0 && !signal_pending(current)) { > /* > * 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. > */ > - lwi = LWI_TIMEOUT_INTR_ALL(HZ, > - ptlrpc_expired_set_void, > - ptlrpc_interrupted_set, set); > - else > + rc = l_wait_event_abortable_timeout(set->set_waitq, > + ptlrpc_check_set(NULL, set), > + HZ); > + 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. > */ > - lwi = LWI_TIMEOUT((timeout ? timeout : 1) * HZ, > - ptlrpc_expired_set_void, 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 && > - 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); > + rc = wait_event_idle_timeout(set->set_waitq, > + ptlrpc_check_set(NULL, set), > + (timeout ? timeout : 1) * HZ); > + if (rc == 0) { > + ptlrpc_expired_set(set); > + rc = -ETIMEDOUT; > + /* > + * 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 (l_fatal_signal_pending(current)) > + ptlrpc_interrupted_set(set); > + } else > + rc = 0; > } > > LASSERT(rc == 0 || rc == -EINTR || rc == -ETIMEDOUT); > @@ -2528,7 +2514,7 @@ static int ptlrpc_unregister_reply(struct ptlrpc_request *request, int async) > return 0; > > /* > - * We have to l_wait_event() whatever the result, to give liblustre > + * We have to wait_event_idle_timeout() whatever the result, to give liblustre > * a chance to run reply_in_callback(), and to make sure we've > * unlinked before returning a req to the pool. > */ > > >