posix-cpu-timers: Move state tracking to struct posix_cputimers

Put it where it belongs and clean up the ifdeffery in fork completely.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20190821192922.743229404@linutronix.de

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 3ea920e8fe7f299091d584ee29a77e8b278ab501..a9e3f69d2db4d1c08d97b740f227ab1d31169cca 100644 (file)
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -77,15 +77,23 @@ struct posix_cputimer_base {
 /**
  * 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
  *
  * 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;
 };
 
 static inline void posix_cputimers_init(struct posix_cputimers *pct)
 {
+       pct->timers_active = 0;
+       pct->expiry_active = 0;
        pct->bases[0].nextevt = U64_MAX;
        pct->bases[1].nextevt = U64_MAX;
        pct->bases[2].nextevt = U64_MAX;

diff --git a/include/linux/sched/cputime.h b/include/linux/sched/cputime.h
index eefa5dff16b4ac5df3c859a495ff550bae0d5b03..6c9f19a33865ab857de84c576718ae95998d7be0 100644 (file)
--- a/include/linux/sched/cputime.h
+++ b/include/linux/sched/cputime.h
@@ -70,7 +70,7 @@ void thread_group_sample_cputime(struct task_struct *tsk, u64 *samples);
  */
 
 /**
- * get_running_cputimer - return &tsk->signal->cputimer if cputimer is running
+ * get_running_cputimer - return &tsk->signal->cputimer if cputimers are active
  *
  * @tsk:       Pointer to target task.
  */
@@ -80,8 +80,11 @@ struct thread_group_cputimer *get_running_cputimer(struct task_struct *tsk)
 {
        struct thread_group_cputimer *cputimer = &tsk->signal->cputimer;
 
-       /* Check if cputimer isn't running. This is accessed without locking. */
-       if (!READ_ONCE(cputimer->running))
+       /*
+        * 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))
                return NULL;
 
        /*

diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 729bd892ee45a11df3ec460ee8ba731e7d6ca47b..88050259c466e1feebd1dcb1ab4ac2ffdf58efa6 100644 (file)
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -57,18 +57,12 @@ struct task_cputime_atomic {
 /**
  * struct thread_group_cputimer - thread group interval timer counts
  * @cputime_atomic:    atomic thread group interval timers.
- * @running:           true when there are timers running and
- *                     @cputime_atomic receives updates.
- * @checking_timer:    true when a thread in the group is in the
- *                     process of checking for thread group timers.
  *
  * This structure contains the version of task_cputime, above, that is
  * used for thread group CPU timer calculations.
  */
 struct thread_group_cputimer {
        struct task_cputime_atomic cputime_atomic;
-       bool running;
-       bool checking_timer;
 };
 
 struct multiprocess_signals {

diff --git a/init/init_task.c b/init/init_task.c
index 7ab773b9b3cd566c504e84d6fc06a0d869705145..d49692a0ec516b0d6b95d6272551903fbbdb14c2 100644 (file)
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -30,8 +30,6 @@ static struct signal_struct init_signals = {
        .posix_timers = LIST_HEAD_INIT(init_signals.posix_timers),
        .cputimer       = {
                .cputime_atomic = INIT_CPUTIME_ATOMIC,
-               .running        = false,
-               .checking_timer = false,
        },
 #endif
        INIT_CPU_TIMERS(init_signals)

diff --git a/kernel/fork.c b/kernel/fork.c
index 52bfe7c20ff61d538e2661073428b72cf4d38bd6..f1228d9f0b11747ca1b030cb831ecb0863e7af9c 100644 (file)
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1517,7 +1517,6 @@ void __cleanup_sighand(struct sighand_struct *sighand)
        }
 }
 
-#ifdef CONFIG_POSIX_TIMERS
 /*
  * Initialize POSIX timer handling for a thread group.
  */
@@ -1528,12 +1527,7 @@ static void posix_cpu_timers_init_group(struct signal_struct *sig)
 
        cpu_limit = READ_ONCE(sig->rlim[RLIMIT_CPU].rlim_cur);
        posix_cputimers_group_init(pct, cpu_limit);
-       if (cpu_limit != RLIM_INFINITY)
-               sig->cputimer.running = true;
 }
-#else
-static inline void posix_cpu_timers_init_group(struct signal_struct *sig) { }
-#endif
 
 static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
 {

diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index ef39a7a4a95c639a47a747fffbeda1d3a3f4fb86..52f4c99c1d6057ba615e530abffa149251828b5c 100644 (file)
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -23,8 +23,10 @@ static void posix_cpu_timer_rearm(struct k_itimer *timer);
 void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit)
 {
        posix_cputimers_init(pct);
-       if (cpu_limit != RLIM_INFINITY)
+       if (cpu_limit != RLIM_INFINITY) {
                pct->bases[CPUCLOCK_PROF].nextevt = cpu_limit * NSEC_PER_SEC;
+               pct->timers_active = true;
+       }
 }
 
 /*
@@ -248,8 +250,9 @@ static void update_gt_cputime(struct task_cputime_atomic *cputime_atomic,
 void thread_group_sample_cputime(struct task_struct *tsk, u64 *samples)
 {
        struct thread_group_cputimer *cputimer = &tsk->signal->cputimer;
+       struct posix_cputimers *pct = &tsk->signal->posix_cputimers;
 
-       WARN_ON_ONCE(!cputimer->running);
+       WARN_ON_ONCE(!pct->timers_active);
 
        proc_sample_cputime_atomic(&cputimer->cputime_atomic, samples);
 }
@@ -269,9 +272,10 @@ void thread_group_sample_cputime(struct task_struct *tsk, u64 *samples)
 static void thread_group_start_cputime(struct task_struct *tsk, u64 *samples)
 {
        struct thread_group_cputimer *cputimer = &tsk->signal->cputimer;
+       struct posix_cputimers *pct = &tsk->signal->posix_cputimers;
 
        /* Check if cputimer isn't running. This is accessed without locking. */
-       if (!READ_ONCE(cputimer->running)) {
+       if (!READ_ONCE(pct->timers_active)) {
                struct task_cputime sum;
 
                /*
@@ -283,13 +287,13 @@ static void thread_group_start_cputime(struct task_struct *tsk, u64 *samples)
                update_gt_cputime(&cputimer->cputime_atomic, &sum);
 
                /*
-                * We're setting cputimer->running without a lock. Ensure
-                * this only gets written to in one operation. We set
-                * running after update_gt_cputime() as a small optimization,
-                * but barriers are not required because update_gt_cputime()
+                * 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
+                * barriers are not required because update_gt_cputime()
                 * can handle concurrent updates.
                 */
-               WRITE_ONCE(cputimer->running, true);
+               WRITE_ONCE(pct->timers_active, true);
        }
        proc_sample_cputime_atomic(&cputimer->cputime_atomic, samples);
 }
@@ -313,9 +317,10 @@ static u64 cpu_clock_sample_group(const clockid_t clkid, struct task_struct *p,
                                  bool start)
 {
        struct thread_group_cputimer *cputimer = &p->signal->cputimer;
+       struct posix_cputimers *pct = &p->signal->posix_cputimers;
        u64 samples[CPUCLOCK_MAX];
 
-       if (!READ_ONCE(cputimer->running)) {
+       if (!READ_ONCE(pct->timers_active)) {
                if (start)
                        thread_group_start_cputime(p, samples);
                else
@@ -834,10 +839,10 @@ static void check_thread_timers(struct task_struct *tsk,
 
 static inline void stop_process_timers(struct signal_struct *sig)
 {
-       struct thread_group_cputimer *cputimer = &sig->cputimer;
+       struct posix_cputimers *pct = &sig->posix_cputimers;
 
-       /* Turn off cputimer->running. This is done without locking. */
-       WRITE_ONCE(cputimer->running, false);
+       /* Turn off the active flag. This is done without locking. */
+       WRITE_ONCE(pct->timers_active, false);
        tick_dep_clear_signal(sig, TICK_DEP_BIT_POSIX_TIMER);
 }
 
@@ -877,17 +882,17 @@ static void check_process_timers(struct task_struct *tsk,
        unsigned long soft;
 
        /*
-        * If cputimer is not running, then there are no active
-        * process wide timers (POSIX 1.b, itimers, RLIMIT_CPU).
+        * If there are no active process wide timers (POSIX 1.b, itimers,
+        * RLIMIT_CPU) nothing to check.
         */
-       if (!READ_ONCE(sig->cputimer.running))
+       if (!READ_ONCE(pct->timers_active))
                return;
 
        /*
         * Signify that a thread is checking for process timers.
         * Write access to this field is protected by the sighand lock.
         */
-       sig->cputimer.checking_timer = true;
+       pct->timers_active = true;
 
        /*
         * Collect the current process totals. Group accounting is active
@@ -933,7 +938,7 @@ static void check_process_timers(struct task_struct *tsk,
        if (expiry_cache_is_inactive(pct))
                stop_process_timers(sig);
 
-       sig->cputimer.checking_timer = false;
+       pct->expiry_active = false;
 }
 
 /*
@@ -1027,39 +1032,41 @@ task_cputimers_expired(const u64 *sample, struct posix_cputimers *pct)
  */
 static inline bool fastpath_timer_check(struct task_struct *tsk)
 {
+       struct posix_cputimers *pct = &tsk->posix_cputimers;
        struct signal_struct *sig;
 
-       if (!expiry_cache_is_inactive(&tsk->posix_cputimers)) {
+       if (!expiry_cache_is_inactive(pct)) {
                u64 samples[CPUCLOCK_MAX];
 
                task_sample_cputime(tsk, samples);
-               if (task_cputimers_expired(samples, &tsk->posix_cputimers))
+               if (task_cputimers_expired(samples, pct))
                        return true;
        }
 
        sig = tsk->signal;
+       pct = &sig->posix_cputimers;
        /*
-        * Check if thread group timers expired when the cputimer is
-        * running and no other thread in the group is already checking
-        * for thread group cputimers. These fields are read without the
-        * sighand lock. However, this is fine because this is meant to
-        * be a fastpath heuristic to determine whether we should try to
-        * acquire the sighand lock to check/handle timers.
+        * Check if thread group timers expired when timers are active and
+        * no other thread in the group is already handling expiry for
+        * thread group cputimers. These fields are read without the
+        * sighand lock. However, this is fine because this is meant to be
+        * a fastpath heuristic to determine whether we should try to
+        * acquire the sighand lock to handle timer expiry.
         *
-        * In the worst case scenario, if 'running' or 'checking_timer' gets
-        * set but the current thread doesn't see the change yet, we'll wait
-        * 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 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.
         */
-       if (READ_ONCE(sig->cputimer.running) &&
-           !READ_ONCE(sig->cputimer.checking_timer)) {
+       if (READ_ONCE(pct->timers_active) && !READ_ONCE(pct->expiry_active)) {
                u64 samples[CPUCLOCK_MAX];
 
                proc_sample_cputime_atomic(&sig->cputimer.cputime_atomic,
                                           samples);
 
-               if (task_cputimers_expired(samples, &sig->posix_cputimers))
+               if (task_cputimers_expired(samples, pct))
                        return true;
        }