From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>,
peterz@infradead.org, oleg@redhat.com, josh@joshtriplett.org,
rostedt@goodmis.org, mathieu.desnoyers@efficios.com,
jiangshanlai@gmail.com, paul.gortmaker@windriver.com,
dmitry.torokhov@gmail.com, linux-kernel@vger.kernel.org
Subject: Re: [RFC v2 1/2] swait: add idle variants which don't contribute to load average
Date: Mon, 19 Jun 2017 10:53:26 -0700 [thread overview]
Message-ID: <20170619175326.GV3721@linux.vnet.ibm.com> (raw)
In-Reply-To: <87k24bsmh4.fsf@xmission.com>
On Fri, Jun 16, 2017 at 03:31:51PM -0500, Eric W. Biederman wrote:
> "Luis R. Rodriguez" <mcgrof@kernel.org> writes:
>
> > There are cases where folks are using an interruptible swait when
> > using kthreads. This is rather confusing given you'd expect
> > interruptible waits to be -- interruptible, but kthreads are not
> > interruptible ! The reason for such practice though is to avoid
> > having these kthreads contribute to the system load average.
> >
> > When systems are idle some kthreads may spend a lot of time blocking if
> > using swait_event_timeout(). This would contribute to the system load
> > average. On systems without preemption this would mean the load average
> > of an idle system is bumped to 2 instead of 0. On systems with PREEMPT=y
> > this would mean the load average of an idle system is bumped to 3
> > instead of 0.
> >
> > This adds proper API using TASK_IDLE to make such goals explicit and
> > avoid confusion.
> >
> > Suggested-by: "Eric W. Biederman" <ebiederm@xmission.com>
> > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> > ---
> > include/linux/swait.h | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/include/linux/swait.h b/include/linux/swait.h
> > index 2c700694d50a..105c70e23286 100644
> > --- a/include/linux/swait.h
> > +++ b/include/linux/swait.h
> > @@ -194,4 +194,29 @@ do { \
> > __ret; \
> > })
> >
> > +#define __swait_event_idle(wq, condition) \
> > + ___swait_event(wq, condition, TASK_IDLE, 0, schedule())
> > +
> > +#define swait_event_idle(wq, condition) \
> > +({ \
> > + int __ret = 0; \
> > + if (!(condition)) \
> > + __ret = __swait_event_idle(wq, condition); \
> > + __ret; \
> > +})
>
> The wait isn't interruptible so a return code doesn't make sense here.
How about as shown below?
Thanx, Paul
------------------------------------------------------------------------
commit 48a96574e9c62ca5e13744e86e2219eb38f080b4
Author: Luis R. Rodriguez <mcgrof@kernel.org>
Date: Thu Jun 15 11:48:19 2017 -0700
swait: add idle variants which don't contribute to load average
There are cases where folks are using an interruptible swait when
using kthreads. This is rather confusing given you'd expect
interruptible waits to be -- interruptible, but kthreads are not
interruptible ! The reason for such practice though is to avoid
having these kthreads contribute to the system load average.
When systems are idle some kthreads may spend a lot of time blocking if
using swait_event_timeout(). This would contribute to the system load
average. On systems without preemption this would mean the load average
of an idle system is bumped to 2 instead of 0. On systems with PREEMPT=y
this would mean the load average of an idle system is bumped to 3
instead of 0.
This adds proper API using TASK_IDLE to make such goals explicit and
avoid confusion.
Suggested-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
[ paulmck: No return value from swait_event_idle per Eric's feedback. ]
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
diff --git a/include/linux/swait.h b/include/linux/swait.h
index c1f9c62a8a50..80d8aee5e7e1 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -169,4 +169,27 @@ do { \
__ret; \
})
+#define __swait_event_idle(wq, condition) \
+ ___swait_event(wq, condition, TASK_IDLE, 0, schedule())
+
+#define swait_event_idle(wq, condition) \
+do { \
+ if (!(condition)) \
+ __swait_event_idle(wq, condition); \
+} while (0)
+
+#define __swait_event_idle_timeout(wq, condition, timeout) \
+ ___swait_event(wq, ___wait_cond_timeout(condition), \
+ TASK_IDLE, timeout, \
+ __ret = schedule_timeout(__ret))
+
+#define swait_event_idle_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!___wait_cond_timeout(condition)) \
+ __ret = __swait_event_idle_timeout(wq, \
+ condition, timeout); \
+ __ret; \
+})
+
#endif /* _LINUX_SWAIT_H */
next prev parent reply other threads:[~2017-06-19 17:53 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-14 23:06 [RFC] rcu: use killable versions of swait Luis R. Rodriguez
2017-06-14 23:43 ` Paul E. McKenney
2017-06-15 15:50 ` Luis R. Rodriguez
2017-06-15 16:22 ` Paul E. McKenney
2017-06-15 16:35 ` Luis R. Rodriguez
2017-06-15 16:55 ` Eric W. Biederman
2017-06-15 18:48 ` [RFC v2 0/2] swait: add idle to make idle-hacks on kthreads explicit Luis R. Rodriguez
2017-06-15 18:48 ` [RFC v2 1/2] swait: add idle variants which don't contribute to load average Luis R. Rodriguez
2017-06-16 0:47 ` Boqun Feng
2017-06-20 21:32 ` Luis R. Rodriguez
2017-06-16 20:31 ` Eric W. Biederman
2017-06-19 17:53 ` Paul E. McKenney [this message]
2017-06-15 18:48 ` [RFC v2 2/2] rcu: use idle versions of swait to make idle-hack clear Luis R. Rodriguez
2017-06-15 21:57 ` [RFC v2 0/2] swait: add idle to make idle-hacks on kthreads explicit Paul E. McKenney
2017-06-15 23:26 ` Luis R. Rodriguez
2017-06-15 23:43 ` Paul E. McKenney
2017-06-16 20:37 ` Eric W. Biederman
2017-06-19 17:54 ` Paul E. McKenney
2017-06-20 21:45 ` [PATCH " Luis R. Rodriguez
2017-06-20 21:45 ` [PATCH 1/2] swait: add idle variants which don't contribute to load average Luis R. Rodriguez
2017-06-20 21:45 ` [PATCH 2/2] rcu: use idle versions of swait to make idle-hack clear Luis R. Rodriguez
2017-06-21 16:48 ` [PATCH 0/2] swait: add idle to make idle-hacks on kthreads explicit Paul E. McKenney
2017-06-21 17:57 ` Luis R. Rodriguez
2017-06-21 18:19 ` Paul E. McKenney
2017-06-15 17:34 ` [RFC] rcu: use killable versions of swait Paul E. McKenney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170619175326.GV3721@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.com \
--cc=dmitry.torokhov@gmail.com \
--cc=ebiederm@xmission.com \
--cc=jiangshanlai@gmail.com \
--cc=josh@joshtriplett.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mcgrof@kernel.org \
--cc=oleg@redhat.com \
--cc=paul.gortmaker@windriver.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.