Re: [PATCH 1/7] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting
(5/7/13 11:24 AM), Frederic Weisbecker wrote: > 2013/5/7 KOSAKI Motohiro : + /* + * After turning over se.sum_exec_runtime to sig->sum_sched_runtime + * in __exit_signal(), we must not account exec_runtime for consistency. + */ + if (unlikely(!tsk->sighand)) + return; >>> >>> Ok, if we want the clock and timer to be consistent, do we also want the >>> same check in >>> account_group_user_time() and account_group_system_time()? The task can >>> still account >>> a tick after autoreaping itself between release_task() and the final >>> schedule(). >> >> You are right. >> >> That said, current the man pages don't describe this linux specific >> extensions. So, nobody >> (glibc, ltp, and me) tested them. Please give me a couple of days. >> I'll test and fix this features >> too. >> >> timer_create(2): http://man7.org/linux/man-pages/man2/timer_create.2.html > > Ah, indeed timer_create() seem to only create CPUCLOCK_SCHED timers. So that > issue with timer_gettime becoming asynchonous with clock_gettime can't happen > with PROF and VIRT clocks > > I see itimers can use those clocks. But there don't seem to be a > similar issue with > getitimer/setitimer as they don't have matching clock reads. OK. I've found PROF and VIRT clock of posix timer have a bug and I could narrow down and fixed it. Please see my next iteration. Thanks. -- 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/
Re: [PATCH 1/7] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting
On Fri, May 10, 2013 at 8:17 PM, Frederic Weisbecker wrote: > On Mon, May 06, 2013 at 11:16:40PM -0400, Olivier Langlois wrote: >> On Tue, 2013-05-07 at 01:47 +0200, Frederic Weisbecker wrote: >> > On Fri, May 03, 2013 at 12:47:42AM -0400, kosaki.motoh...@gmail.com wrote: >> > > From: KOSAKI Motohiro >> > > >> > > When tsk->signal->cputimer->running is 1, signal->cputimer and >> > > tsk->sum_sched_runtime increase at the same pace because update_curr() >> > > increases both accounting. >> > > >> > > However, there is one exception. When thread exiting, __exit_signal() >> > > turns >> > > over task's sum_shced_runtime to sig->sum_sched_runtime, but it doesn't >> > > stop >> > > signal->cputimer accounting. >> > > >> > > This inconsistency makes POSIX timer wake up too early. This patch fixes >> > > it. >> > > >> > > Cc: Thomas Gleixner >> > > Cc: Frederic Weisbecker >> > > Cc: Ingo Molnar >> > > Acked-by: Peter Zijlstra >> > > Signed-off-by: Olivier Langlois >> > > Signed-off-by: KOSAKI Motohiro >> > > --- >> > > kernel/sched/stats.h |7 +++ >> > > 1 files changed, 7 insertions(+), 0 deletions(-) >> > > >> > > diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h >> > > index 2ef90a5..5a0cfc4 100644 >> > > --- a/kernel/sched/stats.h >> > > +++ b/kernel/sched/stats.h >> > > @@ -225,6 +225,13 @@ static inline void >> > > account_group_exec_runtime(struct task_struct *tsk, >> > > if (!cputimer->running) >> > > return; >> > > >> > > + /* >> > > + * After turning over se.sum_exec_runtime to sig->sum_sched_runtime >> > > + * in __exit_signal(), we must not account exec_runtime for >> > > consistency. >> > > + */ > > Please just precise the nature of that consistency: the fact we want > CLOCK_PROCESS_CPUTIME_ID > clock and timer to be consistent. typo? This patch fixes an inconsistency between a thread cputime and a process cputime. How is this? /* * After turning over se.sum_exec_runtime to sig->sum_sched_runtime * in __exit_signal(), a per-thread cputime of the thread will be lost. We * must not account exec_runtime here too because we need to keep * consistent cputime between per-thread and per-process. Otherwise, * the inconsistency is observable when single thread program run. */ -- 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/
Re: [PATCH 1/7] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting
On Mon, May 06, 2013 at 11:16:40PM -0400, Olivier Langlois wrote: > On Tue, 2013-05-07 at 01:47 +0200, Frederic Weisbecker wrote: > > On Fri, May 03, 2013 at 12:47:42AM -0400, kosaki.motoh...@gmail.com wrote: > > > From: KOSAKI Motohiro > > > > > > When tsk->signal->cputimer->running is 1, signal->cputimer and > > > tsk->sum_sched_runtime increase at the same pace because update_curr() > > > increases both accounting. > > > > > > However, there is one exception. When thread exiting, __exit_signal() > > > turns > > > over task's sum_shced_runtime to sig->sum_sched_runtime, but it doesn't > > > stop > > > signal->cputimer accounting. > > > > > > This inconsistency makes POSIX timer wake up too early. This patch fixes > > > it. > > > > > > Cc: Thomas Gleixner > > > Cc: Frederic Weisbecker > > > Cc: Ingo Molnar > > > Acked-by: Peter Zijlstra > > > Signed-off-by: Olivier Langlois > > > Signed-off-by: KOSAKI Motohiro > > > --- > > > kernel/sched/stats.h |7 +++ > > > 1 files changed, 7 insertions(+), 0 deletions(-) > > > > > > diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h > > > index 2ef90a5..5a0cfc4 100644 > > > --- a/kernel/sched/stats.h > > > +++ b/kernel/sched/stats.h > > > @@ -225,6 +225,13 @@ static inline void account_group_exec_runtime(struct > > > task_struct *tsk, > > > if (!cputimer->running) > > > return; > > > > > > + /* > > > + * After turning over se.sum_exec_runtime to sig->sum_sched_runtime > > > + * in __exit_signal(), we must not account exec_runtime for > > > consistency. > > > + */ Please just precise the nature of that consistency: the fact we want CLOCK_PROCESS_CPUTIME_ID clock and timer to be consistent. > > > + if (unlikely(!tsk->sighand)) > > > + return; > > > > Ok, if we want the clock and timer to be consistent, do we also want the > > same check in > > account_group_user_time() and account_group_system_time()? The task can > > still account > > a tick after autoreaping itself between release_task() and the final > > schedule(). > > > If I can reply, I believe that Kosaki refactoring is superior to my > initial proposal because: > > 1. Condition will only be evaluated when cputimer is on (and not slow > down scheduler when it is off) > 2. It handles all the cases when this function is called. There are few > other places outside update_curr() that this function is called. Agreed. Thanks! Acked-by: Frederic Weisbecker -- 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/
Re: [PATCH 1/7] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting
2013/5/7 KOSAKI Motohiro : >>> + /* >>> + * After turning over se.sum_exec_runtime to sig->sum_sched_runtime >>> + * in __exit_signal(), we must not account exec_runtime for >>> consistency. >>> + */ >>> + if (unlikely(!tsk->sighand)) >>> + return; >> >> Ok, if we want the clock and timer to be consistent, do we also want the >> same check in >> account_group_user_time() and account_group_system_time()? The task can >> still account >> a tick after autoreaping itself between release_task() and the final >> schedule(). > > You are right. > > That said, current the man pages don't describe this linux specific > extensions. So, nobody > (glibc, ltp, and me) tested them. Please give me a couple of days. > I'll test and fix this features > too. > > timer_create(2): http://man7.org/linux/man-pages/man2/timer_create.2.html Ah, indeed timer_create() seem to only create CPUCLOCK_SCHED timers. So that issue with timer_gettime becoming asynchonous with clock_gettime can't happen with PROF and VIRT clocks I see itimers can use those clocks. But there don't seem to be a similar issue with getitimer/setitimer as they don't have matching clock reads. Thanks. -- 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/
Re: [PATCH 1/7] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting
On Tue, 2013-05-07 at 01:47 +0200, Frederic Weisbecker wrote: > On Fri, May 03, 2013 at 12:47:42AM -0400, kosaki.motoh...@gmail.com wrote: > > From: KOSAKI Motohiro > > > > When tsk->signal->cputimer->running is 1, signal->cputimer and > > tsk->sum_sched_runtime increase at the same pace because update_curr() > > increases both accounting. > > > > However, there is one exception. When thread exiting, __exit_signal() turns > > over task's sum_shced_runtime to sig->sum_sched_runtime, but it doesn't stop > > signal->cputimer accounting. > > > > This inconsistency makes POSIX timer wake up too early. This patch fixes it. > > > > Cc: Thomas Gleixner > > Cc: Frederic Weisbecker > > Cc: Ingo Molnar > > Acked-by: Peter Zijlstra > > Signed-off-by: Olivier Langlois > > Signed-off-by: KOSAKI Motohiro > > --- > > kernel/sched/stats.h |7 +++ > > 1 files changed, 7 insertions(+), 0 deletions(-) > > > > diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h > > index 2ef90a5..5a0cfc4 100644 > > --- a/kernel/sched/stats.h > > +++ b/kernel/sched/stats.h > > @@ -225,6 +225,13 @@ static inline void account_group_exec_runtime(struct > > task_struct *tsk, > > if (!cputimer->running) > > return; > > > > + /* > > + * After turning over se.sum_exec_runtime to sig->sum_sched_runtime > > + * in __exit_signal(), we must not account exec_runtime for > > consistency. > > + */ > > + if (unlikely(!tsk->sighand)) > > + return; > > Ok, if we want the clock and timer to be consistent, do we also want the same > check in > account_group_user_time() and account_group_system_time()? The task can still > account > a tick after autoreaping itself between release_task() and the final > schedule(). > If I can reply, I believe that Kosaki refactoring is superior to my initial proposal because: 1. Condition will only be evaluated when cputimer is on (and not slow down scheduler when it is off) 2. It handles all the cases when this function is called. There are few other places outside update_curr() that this function is called. -- 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/
Re: [PATCH 1/7] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting
>> + /* >> + * After turning over se.sum_exec_runtime to sig->sum_sched_runtime >> + * in __exit_signal(), we must not account exec_runtime for >> consistency. >> + */ >> + if (unlikely(!tsk->sighand)) >> + return; > > Ok, if we want the clock and timer to be consistent, do we also want the same > check in > account_group_user_time() and account_group_system_time()? The task can still > account > a tick after autoreaping itself between release_task() and the final > schedule(). You are right. That said, current the man pages don't describe this linux specific extensions. So, nobody (glibc, ltp, and me) tested them. Please give me a couple of days. I'll test and fix this features too. timer_create(2): http://man7.org/linux/man-pages/man2/timer_create.2.html -- 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/
Re: [PATCH 1/7] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting
On Fri, May 03, 2013 at 12:47:42AM -0400, kosaki.motoh...@gmail.com wrote: > From: KOSAKI Motohiro > > When tsk->signal->cputimer->running is 1, signal->cputimer and > tsk->sum_sched_runtime increase at the same pace because update_curr() > increases both accounting. > > However, there is one exception. When thread exiting, __exit_signal() turns > over task's sum_shced_runtime to sig->sum_sched_runtime, but it doesn't stop > signal->cputimer accounting. > > This inconsistency makes POSIX timer wake up too early. This patch fixes it. > > Cc: Thomas Gleixner > Cc: Frederic Weisbecker > Cc: Ingo Molnar > Acked-by: Peter Zijlstra > Signed-off-by: Olivier Langlois > Signed-off-by: KOSAKI Motohiro > --- > kernel/sched/stats.h |7 +++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h > index 2ef90a5..5a0cfc4 100644 > --- a/kernel/sched/stats.h > +++ b/kernel/sched/stats.h > @@ -225,6 +225,13 @@ static inline void account_group_exec_runtime(struct > task_struct *tsk, > if (!cputimer->running) > return; > > + /* > + * After turning over se.sum_exec_runtime to sig->sum_sched_runtime > + * in __exit_signal(), we must not account exec_runtime for consistency. > + */ > + if (unlikely(!tsk->sighand)) > + return; Ok, if we want the clock and timer to be consistent, do we also want the same check in account_group_user_time() and account_group_system_time()? The task can still account a tick after autoreaping itself between release_task() and the final schedule(). > + > raw_spin_lock(&cputimer->lock); > cputimer->cputime.sum_exec_runtime += ns; > raw_spin_unlock(&cputimer->lock); > -- > 1.7.1 > -- 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/
[PATCH 1/7] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting
From: KOSAKI Motohiro When tsk->signal->cputimer->running is 1, signal->cputimer and tsk->sum_sched_runtime increase at the same pace because update_curr() increases both accounting. However, there is one exception. When thread exiting, __exit_signal() turns over task's sum_shced_runtime to sig->sum_sched_runtime, but it doesn't stop signal->cputimer accounting. This inconsistency makes POSIX timer wake up too early. This patch fixes it. Cc: Thomas Gleixner Cc: Frederic Weisbecker Cc: Ingo Molnar Acked-by: Peter Zijlstra Signed-off-by: Olivier Langlois Signed-off-by: KOSAKI Motohiro --- kernel/sched/stats.h |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h index 2ef90a5..5a0cfc4 100644 --- a/kernel/sched/stats.h +++ b/kernel/sched/stats.h @@ -225,6 +225,13 @@ static inline void account_group_exec_runtime(struct task_struct *tsk, if (!cputimer->running) return; + /* +* After turning over se.sum_exec_runtime to sig->sum_sched_runtime +* in __exit_signal(), we must not account exec_runtime for consistency. +*/ + if (unlikely(!tsk->sighand)) + return; + raw_spin_lock(&cputimer->lock); cputimer->cputime.sum_exec_runtime += ns; raw_spin_unlock(&cputimer->lock); -- 1.7.1 -- 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/
