Received: by 2002:ac0:a594:0:0:0:0:0 with SMTP id m20-v6csp1758914imm; Fri, 11 May 2018 23:31:05 -0700 (PDT) X-Google-Smtp-Source: AB8JxZqopWLv2uul2WAhcHBh6VNRLpAp+VVmkX2EQfMCbYjJ/SJwBsHuYB8H9BPpMXecD6Q/56xP X-Received: by 2002:a65:508c:: with SMTP id r12-v6mr1719651pgp.185.1526106665367; Fri, 11 May 2018 23:31:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1526106665; cv=none; d=google.com; s=arc-20160816; b=nHBj96S1W2QF4WWGkrW8n8dY1UBnwxiaF3fd2lrqdyBBNKsZhx3BMft8hBe+DFAws7 3a+RFDf973dDyj4yg63RT3RIyC840cOPfYzBICT34iPH/kJJjMnRVz7vKqjNG3sqVdvy /Jf869UcKvvovq+ew3HgD87VzuV0mugNzD4lh+v8uJE6wM/5evYK1DTI+1CNaH8pTGO9 04yxTl8FlnmcDZRD4MdMlzZJMAraw89CsB40BHuo6mfBl7wO3AaI5cfUpMsP1HIZBFLQ 9utM2shry6QnXh6ftPSrP0n0yvHWfqRGvf3Nra3gVn54JjQ+57urJQLcLpcK0uFrWhCP UPzw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-transfer-encoding:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature :arc-authentication-results; bh=U4ccpiB/Yzi8fyZang7NpZjf+Xm3kW5NJuefgN8rLsg=; b=D3nJCZeSXKeJgzLjJBvNFzoPHvd5tsgdSccskjY3VWSxOAk7sDf0pJ+k/Bi7Lm4IId ElLkkZIpOcgJ+kqP9spGF/eQKmQ2kJWBj3VC5dvx9uBINh7wc5fxWC38o8qzgWGoHbu0 bO8Dd3aPMaJxuHTay0bFIlq8Pkw/TrP/CrqyjMeWceSNc8sqQX/UXBBgAZTp/EUaOs6b KPxOjXo/OCUmk6a4GO9DiLHVYNsixZNKKnmYk1FXE3B5VpsBf9Yu7oG6DwAHxcMCO6H6 cwZHBOBpJzpb3wcxQu1HjLiuR2JqHkba+zOF4/VnL+IB57eslNckpdaevCjoraarHFOg GB6Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@joelfernandes-org.20150623.gappssmtp.com header.s=20150623 header.b=H7Q//x6P; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id a15-v6si3808631pgd.531.2018.05.11.23.30.50; Fri, 11 May 2018 23:31:05 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@joelfernandes-org.20150623.gappssmtp.com header.s=20150623 header.b=H7Q//x6P; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750805AbeELGal (ORCPT + 99 others); Sat, 12 May 2018 02:30:41 -0400 Received: from mail-pg0-f65.google.com ([74.125.83.65]:42550 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750749AbeELGaj (ORCPT ); Sat, 12 May 2018 02:30:39 -0400 Received: by mail-pg0-f65.google.com with SMTP id p9-v6so3307348pgc.9 for ; Fri, 11 May 2018 23:30:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=U4ccpiB/Yzi8fyZang7NpZjf+Xm3kW5NJuefgN8rLsg=; b=H7Q//x6PcvwI787RvNoqMaoTUPm3nsLaWE0/ZcgUxXK6U5vgodxImR8dOMe6BnoJ8L 0qed22wWICL+87hVKM1AMUNOeHDVtbD89LTghpah+PpNbBZx5/kTeSS7gbwhryGUMnWF uIkO7CU0WeGFt2ghM5NGimtML5MjkFbb52Do6oIwdLiuDlaukZ7+9MPMaLG1og6YLp6n M1+C+fuBdMroyrLgoMCz0UwR+4r4EzYNa+JAgfSbOiOhL+vxh3uuKH6r0nCTL45li5Q7 y6cZHDWerpitlUfxC0EW+hNiKkGTGMBO3IBxtW+z7xY9sagWAaT5M4OpIkcD2lPC1rmR AgmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=U4ccpiB/Yzi8fyZang7NpZjf+Xm3kW5NJuefgN8rLsg=; b=B0TGVWcEB0efrUiAcp1BR8GmqynWmaTZfX0+ccQRXrl1b/4Pfe5R+ZXAWTfV+mvdGZ 0fp1CQRArKXhyZ+vhNlvESU91/G9RD17J1BNOUSuWIlwKN8CFdjG+doCuC2Bpxu3im4e XMMWu+E2tSy6ZTVxnInmX7Hr9AlFvDI+z/07gHnzDCFmk06YcRg4L9aYcOof//kejoF9 SO8E9N9rgkayK4lP8ehAXVIf1U19cJpRPRjXvnwYe0k5E6IZqTkIlXE+dDIXhOQDkCek UVx5bOSe2Dz5FtABH9V9w8m6eVOPxPVkjb3pam1hrzqRW/iY/wjhIfWTE7+Mt/9DKuZ6 UlFg== X-Gm-Message-State: ALKqPwfkMx3q6Q6tAhF0Wb8cYXy+taIjyRTZkbzC0Ii4r9gEIgp6tNAO XtHXl7Z145ugcLqO2Kx7QNLnqQ== X-Received: by 2002:a65:6449:: with SMTP id s9-v6mr1819981pgv.236.1526106639064; Fri, 11 May 2018 23:30:39 -0700 (PDT) Received: from localhost ([2620:0:1000:1600:3122:ea9c:d178:eb]) by smtp.gmail.com with ESMTPSA id b89-v6sm10602309pfd.85.2018.05.11.23.30.38 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 11 May 2018 23:30:38 -0700 (PDT) Date: Fri, 11 May 2018 23:30:37 -0700 From: Joel Fernandes To: "Paul E. McKenney" Cc: Byungchul Park , jiangshanlai@gmail.com, josh@joshtriplett.org, rostedt@goodmis.org, mathieu.desnoyers@efficios.com, linux-kernel@vger.kernel.org, kernel-team@lge.com, peterz@infradead.org Subject: Re: [PATCH] rcu: Report a quiescent state when it's exactly in the state Message-ID: <20180512063037.GC192642@joelaf.mtv.corp.google.com> References: <1526027434-21237-1-git-send-email-byungchul.park@lge.com> <3af4cec0-4019-e3ac-77f9-8631252fb6da@lge.com> <20180511161746.GX26088@linux.vnet.ibm.com> <20180511224138.GA89902@joelaf.mtv.corp.google.com> <20180512050824.GF26088@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180512050824.GF26088@linux.vnet.ibm.com> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 11, 2018 at 10:08:24PM -0700, Paul E. McKenney wrote: > On Fri, May 11, 2018 at 03:41:38PM -0700, Joel Fernandes wrote: > > On Fri, May 11, 2018 at 09:17:46AM -0700, Paul E. McKenney wrote: > > > On Fri, May 11, 2018 at 09:57:54PM +0900, Byungchul Park wrote: > > > > Hello folks, > > > > > > > > I think I wrote the title in a misleading way. > > > > > > > > Please change the title to something else such as, > > > > "rcu: Report a quiescent state when it's in the state" or, > > > > "rcu: Add points reporting quiescent states where proper" or so on. > > > > > > > > On 2018-05-11 오후 5:30, Byungchul Park wrote: > > > > >We expect a quiescent state of TASKS_RCU when cond_resched_tasks_rcu_qs() > > > > >is called, no matter whether it actually be scheduled or not. However, > > > > >it currently doesn't report the quiescent state when the task enters > > > > >into __schedule() as it's called with preempt = true. So make it report > > > > >the quiescent state unconditionally when cond_resched_tasks_rcu_qs() is > > > > >called. > > > > > > > > > >And in TINY_RCU, even though the quiescent state of rcu_bh also should > > > > >be reported when the tick interrupt comes from user, it doesn't. So make > > > > >it reported. > > > > > > > > > >Lastly in TREE_RCU, rcu_note_voluntary_context_switch() should be > > > > >reported when the tick interrupt comes from not only user but also idle, > > > > >as an extended quiescent state. > > > > > > > > > >Signed-off-by: Byungchul Park > > > > >--- > > > > > include/linux/rcupdate.h | 4 ++-- > > > > > kernel/rcu/tiny.c | 6 +++--- > > > > > kernel/rcu/tree.c | 4 ++-- > > > > > 3 files changed, 7 insertions(+), 7 deletions(-) > > > > > > > > > >diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > > > > >index ee8cf5fc..7432261 100644 > > > > >--- a/include/linux/rcupdate.h > > > > >+++ b/include/linux/rcupdate.h > > > > >@@ -195,8 +195,8 @@ static inline void exit_tasks_rcu_finish(void) { } > > > > > */ > > > > > #define cond_resched_tasks_rcu_qs() \ > > > > > do { \ > > > > >- if (!cond_resched()) \ > > > > >- rcu_note_voluntary_context_switch_lite(current); \ > > > > >+ rcu_note_voluntary_context_switch_lite(current); \ > > > > >+ cond_resched(); \ > > > > > > Ah, good point. > > > > > > Peter, I have to ask... Why is "cond_resched()" considered a preemption > > > while "schedule()" is not? > > > > Infact something interesting I inferred from the __schedule loop related to > > your question: > > > > switch_count can either be set to prev->invcsw or prev->nvcsw. If we can > > assume that switch_count reflects whether the context switch is involuntary > > or voluntary, > > > > task-running-state preempt switch_count > > 0 (running) 1 involuntary > > 0 0 involuntary > > 1 0 voluntary > > 1 1 involuntary > > > > According to the above table, both the task's running state and the preempt > > parameter to __schedule should be used together to determine if the switch is > > a voluntary one or not. > > > > So this code in rcu_note_context_switch should really be: > > if (!preempt && !(current->state & TASK_RUNNING)) I should have writte here- !preempt && current->state > > rcu_note_voluntary_context_switch_lite(current); > > > > According to the above table, cond_resched always classifies as an > > involuntary switch which makes sense to me. Even though cond_resched is > > explicitly called, its still sort of involuntary in the sense its not called > > into the scheduler for sleeping, but rather for seeing if something else can > > run instead (a preemption point). Infact none of the task deactivation in the > > __schedule loop will run if cond_resched is used. > > > > I agree that if schedule was called directly but with TASK_RUNNING=1, then > > that could probably be classified an involuntary switch too... > > > > Also since we're deciding to call rcu_note_voluntary_context_switch_lite > > unconditionally, then IMO this comment on that macro: > > > > /* > > * Note a voluntary context switch for RCU-tasks benefit. This is a > > * macro rather than an inline function to avoid #include hell. > > */ > > #ifdef CONFIG_TASKS_RCU > > #define rcu_note_voluntary_context_switch_lite(t) > > > > Should be changed to: > > > > /* > > * Note a attempt to perform a voluntary context switch for RCU-tasks > > * benefit. This is called even in situations where a context switch > > * didn't really happen even though it was requested. This is a > > * macro rather than an inline function to avoid #include hell. > > */ > > #ifdef CONFIG_TASKS_RCU > > #define rcu_note_voluntary_context_switch_lite(t) > > > > Right? > > > > Correct me if I'm wrong about anything, thanks, > > The starting point for me is that Tasks RCU is a special-purpose mechanism > for freeing trampolines in PREEMPT=y kernels. The approach is to arrange > for the trampoline to be inaccessible to future execution, wait for a > tasks-RCU grace period, then free the trampoline. So a tasks-RCU grace > period must wait until all tasks have spent at least some time outside > of a trampoline. My understanding is that trampolines cannot contain > preemption points, such as cond_resched() and cond_resched_tasks_rcu_qs(), > so we want to count them as quiescent states regardless of whether or > not any associated context switch is counted as involuntary. > > What situations lead to the second line of your table above? > The sched_yield() system call, but trampolines don't do system calls, > either, as far as I know. > > So it looks to me like that test can leave out the TASK_RUNNING check. I don't know much about tasks-RCU to comment more, sorry. Probably a few more reading nights for me to catch up with that. Its possible the check is not needed and tasks-RCU can survive without it, but I was thinking from a correctness and future-proofing stand point... I generally don't like inconsistencies. The check in the __schedule loop is as: if (!preempt && prev->state) { .... // switch_count = voluntary context switch counter pointer .... } else { .... // switch_count = involuntary context switch counter pointer .... } // context switch really happening if (prev != next) { .... ++switch_count; } The first conditional if (!preempt...) above is what I was referring to which also checks the state. Also this issue aside, I was more trying to answer your question about why schedule() is or isn't a preemption point, by sharing the table but I possibly caused more confusion, sorry :-(. I'll let Peter and Steven chime in since they know more than me about that and will just shutup and listen instead of being more noisy.. :-D thanks, - Joel