Messages in this thread | | | Date | Wed, 15 Apr 2015 09:33:41 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 2/3] sched, timer: Use atomics for thread_group_cputimer to improve scalability |
| |
* Jason Low <jason.low2@hp.com> wrote:
> While running a database workload, we found a scalability issue with itimers. > > Much of the problem was caused by the thread_group_cputimer spinlock.
So I'm fine with the basic principle, but in the hope that maybe posix-cpu-timers will grow similar optimizations in the future, it would help to have the new data type factored out better, not open-coded:
> struct thread_group_cputimer { > - struct task_cputime cputime; > + atomic64_t utime; > + atomic64_t stime; > + atomic64_t sum_exec_runtime; > int running; > - raw_spinlock_t lock; > };
So after your changes we still have a separate:
struct task_cputime { cputime_t utime; cputime_t stime; unsigned long long sum_exec_runtime; };
Which then weirdly overlaps with a different structure on a different abstraction level:
struct thread_group_cputimer { atomic64_t utime; atomic64_t stime; atomic64_t sum_exec_runtime; int running; };
So I think it would be more obvious what's going on if we introduced an atomic task_cputime structure:
struct task_cputime_atomic { atomic64_t utime; atomic64_t stime; atomic64_t sum_exec_runtime; };
and put that into 'struct thread_group_cputimer':
struct thread_group_cputimer { struct task_cputime_atomic cputime_atomic; int running; };
Maybe even factor out the main update and reading methods into expressively named helper inlines?
Thanks,
Ingo
| |