From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick Farrell Date: Mon, 29 Oct 2018 01:35:46 +0000 Subject: [lustre-devel] [PATCH 12/28] lustre: ptlrpc: do not wakeup every second In-Reply-To: <87va5l39hl.fsf@notabene.neil.brown.name> References: <1539543498-29105-1-git-send-email-jsimmons@infradead.org> <1539543498-29105-13-git-send-email-jsimmons@infradead.org>, <87va5l39hl.fsf@notabene.neil.brown.name> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: lustre-devel@lists.lustre.org Neil, Does your statement imply this would spin? It definitely doesn?t just spin (that behavior in a main ?wait for work? spot of a (depending on settings) ~per-CPU daemon would render systems unusable and this patch has been in testing for a while). So what is the detailed behavior of a ?timeout that expires immediately?? - Patrick ________________________________ From: lustre-devel on behalf of NeilBrown Sent: Sunday, October 28, 2018 7:03:02 PM To: James Simmons; Andreas Dilger; Oleg Drokin Cc: Lustre Development List Subject: Re: [lustre-devel] [PATCH 12/28] lustre: ptlrpc: do not wakeup every second On Sun, Oct 14 2018, James Simmons wrote: > From: Alex Zhuravlev > > Even if there are no RPC requests on the set, there is no need to > wake up every second. The thread is woken up when a request is added > to the set or when the STOP bit is set, so it is sufficient to only > wake up when there are requests on the set to worry about. > > Signed-off-by: Alex Zhuravlev > WC-bug-id: https://jira.whamcloud.com/browse/LU-9660 > Reviewed-on: https://review.whamcloud.com/28776 > Reviewed-by: Andreas Dilger > Reviewed-by: Patrick Farrell > Reviewed-by: Oleg Drokin > Signed-off-by: James Simmons > --- > drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c > index c201a88..5b4977b 100644 > --- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c > +++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c > @@ -371,7 +371,7 @@ static int ptlrpcd_check(struct lu_env *env, struct ptlrpcd_ctl *pc) > } > } > > - return rc; > + return rc || test_bit(LIOD_STOP, &pc->pc_flags); > } > > /** > @@ -441,7 +441,7 @@ static int ptlrpcd(void *arg) > lu_context_enter(env.le_ses); > if (wait_event_idle_timeout(set->set_waitq, > ptlrpcd_check(&env, pc), > - (timeout ? timeout : 1) * HZ) == 0) > + timeout * HZ) == 0) > ptlrpc_expired_set(set); This is incorrect. A timeout of zero means the timeout happens after zero jiffies (immediately), it doesn't mean there is no timeout. If we want a "timeout" of zero to mean "Wait forever", we need something like: wait_event_idle_timeout(....., timeout ? (timeout * HZ) : MAX_SCHEDULE_TIMEOUT) == 0 I've updated the patch accordingly. Thanks, NeilBrown > > lu_context_exit(&env.le_ctx); > -- > 1.8.3.1 -------------- next part -------------- An HTML attachment was scrubbed... URL: