Messages in this thread | | | From | John Stultz <> | Date | Fri, 20 Jan 2017 14:48:23 -0800 | Subject | Re: [PATCH] omit POSIX timer stuff from task_struct when disabled |
| |
On Fri, Jan 20, 2017 at 2:46 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > On Fri, 20 Jan 2017, John Stultz wrote: > >> On Mon, Jan 9, 2017 at 10:49 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: >> > When CONFIG_POSIX_TIMERS is disabled, it is preferable to remove related >> > structures from struct task_struct and struct signal_struct as they >> > won't contain anything useful and shouldn't be relied upon by mistake. >> > Code still referencing those structures is also disabled here. >> > >> > Signed-off-by: Nicolas Pitre <nico@linaro.org> >> > >> [snip] >> > diff --git a/kernel/fork.c b/kernel/fork.c >> > index 11c5c8ab82..8e333e55a9 100644 >> > --- a/kernel/fork.c >> > +++ b/kernel/fork.c >> > @@ -1309,6 +1309,7 @@ void __cleanup_sighand(struct sighand_struct *sighand) >> > */ >> > static void posix_cpu_timers_init_group(struct signal_struct *sig) >> > { >> > +#ifdef CONFIG_POSIX_TIMERS >> > unsigned long cpu_limit; >> > >> > cpu_limit = READ_ONCE(sig->rlim[RLIMIT_CPU].rlim_cur); >> > @@ -1321,6 +1322,7 @@ static void posix_cpu_timers_init_group(struct signal_struct *sig) >> > INIT_LIST_HEAD(&sig->cpu_timers[0]); >> > INIT_LIST_HEAD(&sig->cpu_timers[1]); >> > INIT_LIST_HEAD(&sig->cpu_timers[2]); >> > +#endif >> > } >> >> So apologies for not catching this earlier. I was just queuing this >> up, when I noticed the style issue here. >> >> Aren't in-function ifdefs frowned upon? Wouldn't it be better to do: >> #ifdef CONFIG_POSIX_TIMERS >> static void posix_cpu_timers_init_group(struct signal_struct *sig) >> { >> ... >> } >> #else >> static void posix_cpu_timers_init_group(struct signal_struct *sig) {} >> #endif >> >> And similar for most of the ifdef'ed out functions in this patch? > > Well... I don't mind either ways. In this case those functions are > rather small and doing it the way you suggest doubles the number of > added lines in this hunk. That's why I opted for the current style. > > Just tell me if you prefer that I respin the patch and I'll do it.
Yea. I'm not so finicky but I'm sure I'll probably get yelled at if I pass it on, so if you don't mind respinning it, I'll get it applied for testing here quickly :)
thanks -john
| |