All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, Oleg Nesterov <oleg@redhat.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	John Stultz <john.stultz@linaro.org>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: [patch V2 2/5] posix-cpu-timers: Convert the flags to a bitmap
Date: Thu, 16 Jul 2020 22:19:25 +0200	[thread overview]
Message-ID: <20200716202044.625081169@linutronix.de> (raw)
In-Reply-To: 20200716201923.228696399@linutronix.de

For splitting the posix cpu timer code into an interrupt context check and
the actual expiry and signal handling it's required to be able to set and
test for the flags atomically.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/posix-timers.h   |   16 +++++++--------
 include/linux/sched/cputime.h  |    2 -
 kernel/time/posix-cpu-timers.c |   43 ++++++++++++++++++-----------------------
 3 files changed, 28 insertions(+), 33 deletions(-)

--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -109,20 +109,20 @@ struct posix_cputimer_base {
 	struct timerqueue_head	tqhead;
 };
 
+enum {
+	CPUTIMERS_ACTIVE,
+	CPUTIMERS_EXPIRING,
+};
+
 /**
  * posix_cputimers - Container for posix CPU timer related data
- * @bases:		Base container for posix CPU clocks
- * @timers_active:	Timers are queued.
- * @expiry_active:	Timer expiry is active. Used for
- *			process wide timers to avoid multiple
- *			task trying to handle expiry concurrently
- *
+ * @bases:	Base container for posix CPU clocks
+ * @flags:	Flags for various CPUTIMERS_* states
  * Used in task_struct and signal_struct
  */
 struct posix_cputimers {
 	struct posix_cputimer_base	bases[CPUCLOCK_MAX];
-	unsigned int			timers_active;
-	unsigned int			expiry_active;
+	unsigned long			flags;
 };
 
 static inline void posix_cputimers_init(struct posix_cputimers *pct)
--- a/include/linux/sched/cputime.h
+++ b/include/linux/sched/cputime.h
@@ -84,7 +84,7 @@ struct thread_group_cputimer *get_runnin
 	 * Check whether posix CPU timers are active. If not the thread
 	 * group accounting is not active either. Lockless check.
 	 */
-	if (!READ_ONCE(tsk->signal->posix_cputimers.timers_active))
+	if (!test_bit(CPUTIMERS_ACTIVE, &tsk->signal->posix_cputimers.flags))
 		return NULL;
 
 	/*
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -25,7 +25,7 @@ void posix_cputimers_group_init(struct p
 	posix_cputimers_init(pct);
 	if (cpu_limit != RLIM_INFINITY) {
 		pct->bases[CPUCLOCK_PROF].nextevt = cpu_limit * NSEC_PER_SEC;
-		pct->timers_active = true;
+		set_bit(CPUTIMERS_ACTIVE, &pct->flags);
 	}
 }
 
@@ -269,7 +269,7 @@ void thread_group_sample_cputime(struct
 	struct thread_group_cputimer *cputimer = &tsk->signal->cputimer;
 	struct posix_cputimers *pct = &tsk->signal->posix_cputimers;
 
-	WARN_ON_ONCE(!pct->timers_active);
+	WARN_ON_ONCE(!test_bit(CPUTIMERS_ACTIVE, &pct->flags));
 
 	proc_sample_cputime_atomic(&cputimer->cputime_atomic, samples);
 }
@@ -292,7 +292,7 @@ static void thread_group_start_cputime(s
 	struct posix_cputimers *pct = &tsk->signal->posix_cputimers;
 
 	/* Check if cputimer isn't running. This is accessed without locking. */
-	if (!READ_ONCE(pct->timers_active)) {
+	if (!test_bit(CPUTIMERS_ACTIVE, &pct->flags)) {
 		struct task_cputime sum;
 
 		/*
@@ -304,13 +304,12 @@ static void thread_group_start_cputime(s
 		update_gt_cputime(&cputimer->cputime_atomic, &sum);
 
 		/*
-		 * We're setting timers_active without a lock. Ensure this
-		 * only gets written to in one operation. We set it after
-		 * update_gt_cputime() as a small optimization, but
+		 * We're setting CPUTIMERS_ACTIVE without a lock. We set it
+		 * after update_gt_cputime() as a small optimization, but
 		 * barriers are not required because update_gt_cputime()
 		 * can handle concurrent updates.
 		 */
-		WRITE_ONCE(pct->timers_active, true);
+		set_bit(CPUTIMERS_ACTIVE, &pct->flags);
 	}
 	proc_sample_cputime_atomic(&cputimer->cputime_atomic, samples);
 }
@@ -335,7 +334,7 @@ static u64 cpu_clock_sample_group(const
 	struct posix_cputimers *pct = &p->signal->posix_cputimers;
 	u64 samples[CPUCLOCK_MAX];
 
-	if (!READ_ONCE(pct->timers_active)) {
+	if (!test_bit(CPUTIMERS_ACTIVE, &pct->flags)) {
 		if (start)
 			thread_group_start_cputime(p, samples);
 		else
@@ -862,7 +861,7 @@ static inline void stop_process_timers(s
 	struct posix_cputimers *pct = &sig->posix_cputimers;
 
 	/* Turn off the active flag. This is done without locking. */
-	WRITE_ONCE(pct->timers_active, false);
+	clear_bit(CPUTIMERS_ACTIVE, &pct->flags);
 	tick_dep_clear_signal(sig, TICK_DEP_BIT_POSIX_TIMER);
 }
 
@@ -906,16 +905,11 @@ static void check_process_timers(struct
 	 * RLIMIT_CPU) nothing to check. Also skip the process wide timer
 	 * processing when there is already another task handling them.
 	 */
-	if (!READ_ONCE(pct->timers_active) || pct->expiry_active)
+	if (!test_bit(CPUTIMERS_ACTIVE, &pct->flags) ||
+	    test_and_set_bit(CPUTIMERS_EXPIRING, &pct->flags))
 		return;
 
 	/*
-	 * Signify that a thread is checking for process timers.
-	 * Write access to this field is protected by the sighand lock.
-	 */
-	pct->expiry_active = true;
-
-	/*
 	 * Collect the current process totals. Group accounting is active
 	 * so the sample can be taken directly.
 	 */
@@ -959,7 +953,7 @@ static void check_process_timers(struct
 	if (expiry_cache_is_inactive(pct))
 		stop_process_timers(sig);
 
-	pct->expiry_active = false;
+	clear_bit(CPUTIMERS_EXPIRING, &pct->flags);
 }
 
 /*
@@ -1057,14 +1051,15 @@ static inline bool fastpath_timer_check(
 	 * a fastpath heuristic to determine whether we should try to
 	 * acquire the sighand lock to handle timer expiry.
 	 *
-	 * In the worst case scenario, if concurrently timers_active is set
-	 * or expiry_active is cleared, but the current thread doesn't see
-	 * the change yet, the timer checks are delayed until the next
-	 * thread in the group gets a scheduler interrupt to handle the
-	 * timer. This isn't an issue in practice because these types of
-	 * delays with signals actually getting sent are expected.
+	 * In the worst case scenario, if concurrently CPUTIMERS_ACTIVE is
+	 * set or CPUTIMERS_EXPIRING is cleared, but the current thread
+	 * doesn't see the change yet, the timer checks are delayed until
+	 * the next thread in the group gets a scheduler interrupt to
+	 * handle the timer. This isn't an issue in practice because these
+	 * types of delays with signals actually getting sent are expected.
 	 */
-	if (READ_ONCE(pct->timers_active) && !READ_ONCE(pct->expiry_active)) {
+	if (test_bit(CPUTIMERS_ACTIVE, &pct->flags) &&
+	    !test_bit(CPUTIMERS_EXPIRING, &pct->flags)) {
 		u64 samples[CPUCLOCK_MAX];
 
 		proc_sample_cputime_atomic(&sig->cputimer.cputime_atomic,


  parent reply	other threads:[~2020-07-16 20:22 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 20:19 [patch V2 0/5] posix-cpu-timers: Move expiry into task work context Thomas Gleixner
2020-07-16 20:19 ` [patch V2 1/5] posix-cpu-timers: Split run_posix_cpu_timers() Thomas Gleixner
2020-07-16 20:19 ` Thomas Gleixner [this message]
2020-07-21 12:34   ` [patch V2 2/5] posix-cpu-timers: Convert the flags to a bitmap Frederic Weisbecker
2020-07-21 16:10     ` Thomas Gleixner
2020-07-21 16:23       ` David Laight
2020-07-21 18:30         ` Thomas Gleixner
2020-07-16 20:19 ` [patch V2 3/5] posix-cpu-timers: Provide mechanisms to defer timer handling to task_work Thomas Gleixner
2020-07-16 22:50   ` Peter Zijlstra
2020-07-17 18:37     ` Thomas Gleixner
2020-07-23  1:03     ` Frederic Weisbecker
2020-07-23  8:32       ` Thomas Gleixner
2020-07-23 12:15         ` Frederic Weisbecker
2020-07-16 22:54   ` Peter Zijlstra
2020-07-17 18:38     ` Thomas Gleixner
2020-07-19 19:33       ` Thomas Gleixner
2020-07-21 18:50         ` Thomas Gleixner
2020-07-17 17:26   ` Oleg Nesterov
2020-07-17 18:35     ` Thomas Gleixner
2020-07-16 20:19 ` [patch V2 4/5] posix-cpu-timers: Expiry timers directly when in task work context Thomas Gleixner
2020-07-16 20:19 ` [patch V2 5/5] x86: Select POSIX_CPU_TIMERS_TASK_WORK Thomas Gleixner

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=20200716202044.625081169@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=ebiederm@xmission.com \
    --cc=frederic@kernel.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=x86@kernel.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.