On Mon, 19 Aug 2019, Ingo Molnar wrote: > * Thomas Gleixner <t...@linutronix.de> wrote: > > struct posix_cputimers { > > - struct task_cputime cputime_expires; > > - struct list_head cpu_timers[CPUCLOCK_MAX]; > > + /* Temporary union until all users are cleaned up */ > > + union { > > + struct task_cputime cputime_expires; > > + u64 expiries[CPUCLOCK_MAX]; > > + }; > > + struct list_head cpu_timers[CPUCLOCK_MAX]; > > }; > > Could we please name this first_expiry[] or such, to make it clear that > this is cached value of the first expiry of all timers of this process, > instead of the rather vague 'expiries[]' naming? > > Also, while at it, after the above temporary transition union, the final > structure becomes: > > struct posix_cputimers { > u64 expiries[CPUCLOCK_MAX]; > struct list_head cpu_timers[CPUCLOCK_MAX]; > }; > > Wouldn't it be more natural and easier to read to have the list head and > the expiry together: > > struct posix_cputimer_list { > u64 first_expiry; > struct list_head list; > }; > > struct posix_cputimers { > struct posix_cputimer_list timers[CPUCLOCK_MAX]; > }; > > ? > > This makes the array structure rather clear and the first_expiry field > mostly self-documenting.
I kept the odd named expiries for the temporary union and then after the patch which removes the abused struct task_cputime, I applied a separate cleanup which looks similar to the above. Just the names are a bit different and more aligned to what we have in hrtimers: struct posix_cputimer_base { u64 nextevt; struct timerqueue_head tqhead; }; and then have struct posix_cputimers { struct posix_cputimer_base bases[CPUCLOCK_MAX]; }; I'll send out a new version after doing some more testing. Thanks, tglx