From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: ACJfBousSTW5L+gFpYm/utPT24GTZsj5nuHivOhbV2Z2UOpiEeGXb3v9P2diyDkZmR5o/wytLiQN ARC-Seal: i=1; a=rsa-sha256; t=1516203191; cv=none; d=google.com; s=arc-20160816; b=MZLY7lU0e1U/8Eky+OE72xKCwTNOYmpulY3ip8Vm46O5YBxanaUv6f+F4JSEA++XWM VOJP60iV740wLD39Db5V0TDsUzQZ2sJ/yljRMrfFmJ66Y/CKq9563tkW3uiGq5Ey/gWg aslRQ5Sn4nr1hZj0iwVF+dtX1e0//Zz6oKvDDLGXjHJhL5eN20PaYc9iieMb0rJc3gIB PBA/SJ9+DFgXoMtoEe265spj7s292t4l1inGFrfFVAwT/xI+DbcXuTNKE0W5de2Y3t43 S6CJjYZyw7k0Hq5bZeAQ5UNB0OY3BCf44ngQlmjmAn2RsqFG396ZFCaxvmXN9ALeSgDN SfaA== 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=v+xs8mrEf1JZ+lJijG4F9DhTUuQXsLcttfTxR+jdAQM=; b=JXphtqD+DoyQsylRZpYsrVOE8c0nFefDGnkjbLE6ahJDrfFniEJsGUO4TFTcIjp5lS lQsf+VZON8xjGpo1I52LxdvaKsxY2IsSpr8SidZelfoNtyeHpBkUsWeQXDzLzs14FZU7 4aXYD2wWH5Tls+7xlbKiW3UgJtolCthkWrpsa+vENktRe6jmt3f9Tb2DOulbHsVFs1Xw taXA+pZnXaripusX86SwDhDvdeBlljY9hJerDecKx2BTv8m1ce1+djFUCanoEQljla6z YXhThfqGZZEaHhum/qllpMtDL0eQM0T6fqclBUNo8+z/+BwGS1EETGw18Ggh6CR6f00d tgGw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@infradead.org header.s=casper.20170209 header.b=V/dsGkRZ; 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=V/dsGkRZ; 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:33:07 +0000 (GMT) From: James Simmons To: NeilBrown cc: Oleg Drokin , Andreas Dilger , Greg Kroah-Hartman , lkml , lustre Subject: Re: [PATCH 11/19] staging: lustre: remove back_to_sleep() In-Reply-To: <151538209372.23920.9644158038491397506.stgit@noble> Message-ID: References: <151538168618.23920.8261096424342988792.stgit@noble> <151538209372.23920.9644158038491397506.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_153307_806505_660AE022 X-CRM114-Status: GOOD ( 18.60 ) 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?1588993435876523612?= X-GMAIL-MSGID: =?utf-8?q?1589854278071532659?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: > When 'back_to_sleep()' is passed as the 'timeout' function, > the effect is to wait indefinitely for the event, polling > once after the timeout. > If LWI_ON_SIGNAL_NOOP is given, then after the timeout > we allow fatal signals to interrupt the wait. > > Make this more obvious in both places "back_to_sleep()" is > used but using two explicit sleeps. > > The code in ptlrpcd_add_req() looks odd - why not just have one > wait_event_idle()? However I believe this is a faithful > transformation of the existing code. Reviewed-by: James Simmons > Signed-off-by: NeilBrown > --- > drivers/staging/lustre/lustre/include/lustre_lib.h | 4 ---- > drivers/staging/lustre/lustre/ptlrpc/import.c | 11 ++++++----- > drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c | 9 +++++---- > 3 files changed, 11 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/include/lustre_lib.h b/drivers/staging/lustre/lustre/include/lustre_lib.h > index b2a64d0e682c..1939e959b92a 100644 > --- a/drivers/staging/lustre/lustre/include/lustre_lib.h > +++ b/drivers/staging/lustre/lustre/include/lustre_lib.h > @@ -140,10 +140,6 @@ void target_send_reply(struct ptlrpc_request *req, int rc, int fail_id); > * XXX nikita: some ptlrpc daemon threads have races of that sort. > * > */ > -static inline int back_to_sleep(void *arg) > -{ > - return 0; > -} > > #define LWI_ON_SIGNAL_NOOP ((void (*)(void *))(-1)) > > diff --git a/drivers/staging/lustre/lustre/ptlrpc/import.c b/drivers/staging/lustre/lustre/ptlrpc/import.c > index 5d62c9de27eb..faf0f606f013 100644 > --- a/drivers/staging/lustre/lustre/ptlrpc/import.c > +++ b/drivers/staging/lustre/lustre/ptlrpc/import.c > @@ -1496,7 +1496,6 @@ int ptlrpc_disconnect_import(struct obd_import *imp, int noclose) > } > > if (ptlrpc_import_in_recovery(imp)) { > - struct l_wait_info lwi; > long timeout; > > if (AT_OFF) { > @@ -1510,10 +1509,12 @@ int ptlrpc_disconnect_import(struct obd_import *imp, int noclose) > timeout = at_get(&imp->imp_at.iat_service_estimate[idx]) * HZ; > } > > - lwi = LWI_TIMEOUT_INTR(cfs_timeout_cap(timeout), > - back_to_sleep, LWI_ON_SIGNAL_NOOP, NULL); > - rc = l_wait_event(imp->imp_recovery_waitq, > - !ptlrpc_import_in_recovery(imp), &lwi); > + if (wait_event_idle_timeout(imp->imp_recovery_waitq, > + !ptlrpc_import_in_recovery(imp), > + cfs_timeout_cap(timeout)) == 0) > + l_wait_event_abortable( > + imp->imp_recovery_waitq, > + !ptlrpc_import_in_recovery(imp)); > } > > spin_lock(&imp->imp_lock); > diff --git a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c > index dad2f9290f70..437b4b2a9072 100644 > --- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c > +++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c > @@ -230,12 +230,13 @@ void ptlrpcd_add_req(struct ptlrpc_request *req) > > spin_lock(&req->rq_lock); > if (req->rq_invalid_rqset) { > - struct l_wait_info lwi = LWI_TIMEOUT(5 * HZ, > - back_to_sleep, NULL); > - > req->rq_invalid_rqset = 0; > spin_unlock(&req->rq_lock); > - l_wait_event(req->rq_set_waitq, !req->rq_set, &lwi); > + if (wait_event_idle_timeout(req->rq_set_waitq, > + !req->rq_set, > + 5 * HZ) == 0) > + wait_event_idle(req->rq_set_waitq, > + !req->rq_set); > } else if (req->rq_set) { > /* If we have a valid "rq_set", just reuse it to avoid double > * linked. > > >