On Mon, Oct 19, 2015 at 05:41:08PM -0700, Davidlohr Bueso wrote: > On Tue, 20 Oct 2015, Frederic Weisbecker wrote: > > >- * @checking_timer: true when a thread in the group is in the > >- * process of checking for thread group timers. > >- * > >+ * @state: flags describing the current state of the cputimer. > >+ * CPUTIMER_STATE_RUNNING bit means the timers is elapsing. > > s/timers/timer > > >+ * CPUTIMER_STATE_CHECKING bit means that the cputimer has > >+ * expired and a thread in the group is checking the > >+ * callback list. > > These comments might be better served when defining CPUTIMER_STATE_*
If it was defined as an emum I'd agree but here state is defined as an int (whose size is more readable in a struct than enum) and it's not obvious what kind of values it can take if we don't define them here. > > [...] > > >@@ -606,7 +606,7 @@ bool posix_cpu_timers_can_stop_tick(struct task_struct > >*tsk) > > return false; > > > > /* Check if cputimer is running. This is accessed without locking. */ > >- if (READ_ONCE(tsk->signal->cputimer.running)) > >+ if (READ_ONCE(tsk->signal->cputimer.state)) > > return false; > > Could we have cases, such as the above, where .state is set to > CPUTIMER_STATE_CHECKING > and therefore the check is not equivalent? Nope we shouldn't. I added a WARN_ONCE somewhere to perform some related sanity checks. I could add more if needed. Thanks. > Thanks, > Davidlohr -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
