linux-kernel - Re: [RFC PATCH v1 1/9] timer: fix circular header dependency

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5c9530da-f5c8-e0b2-61ab-294a0da7f3fc@c-s.fr>
Date:   Tue, 25 Sep 2018 07:34:15 +0200
From:   Christophe LEROY <christophe.leroy@....fr>
To:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        aneesh.kumar@...ux.vnet.ibm.com, npiggin@...il.com,
        Thomas Gleixner <tglx@...utronix.de>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        John Stultz <john.stultz@...aro.org>,
        Stephen Boyd <sboyd@...nel.org>
Cc:     linux-fsdevel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v1 1/9] timer: fix circular header dependency



Le 24/09/2018 à 17:52, Christophe Leroy a écrit :
> When switching powerpc to CONFIG_THREAD_INFO_IN_TASK, include/sched.h
> has to be included in asm/smp.h for the following change, in order
> to avoid uncomplete definition of task_struct:
> 
> -#define raw_smp_processor_id() (current_thread_info()->cpu)
> +#define raw_smp_processor_id() (current->cpu)
> 
> But this generates the following compilation error, due to circular
> header dependency.
> 
>    CC      kernel/time/alarmtimer.o
> In file included from ./arch/powerpc/include/asm/smp.h:31,
>                   from ./include/linux/smp.h:64,
>                   from ./include/linux/percpu.h:7,
>                   from ./include/linux/hrtimer.h:22,
>                   from kernel/time/alarmtimer.c:19:
> ./include/linux/sched.h:558:19: error: field 'dl_timer' has incomplete type
>    struct hrtimer   dl_timer;
>                     ^~~~~~~~
> ./include/linux/sched.h:567:17: error: field 'inactive_timer' has incomplete type
>    struct hrtimer inactive_timer;
>                   ^~~~~~~~~~~~~~
> make[1]: *** [kernel/time/alarmtimer.o] Error 1
> make: *** [kernel/time/alarmtimer.o] Error 2
> 
>    CC      fs/timerfd.o
> In file included from ./arch/powerpc/include/asm/smp.h:31,
>                   from ./include/linux/smp.h:64,
>                   from ./include/linux/percpu.h:7,
>                   from ./include/linux/hrtimer.h:22,
>                   from ./include/linux/alarmtimer.h:6,
>                   from fs/timerfd.c:12:
> ./include/linux/sched.h:558:19: error: field 'dl_timer' has incomplete type
>    struct hrtimer   dl_timer;
>                     ^~~~~~~~
> ./include/linux/sched.h:567:17: error: field 'inactive_timer' has incomplete type
>    struct hrtimer inactive_timer;
>                   ^~~~~~~~~~~~~~
> make[1]: *** [fs/timerfd.o] Error 1
> make: *** [fs/timerfd.o] Error 2
> 
> This patch fixes it by including linux/hrtimer.h after linux/sched.h
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@....fr>
> ---
>   Should it be fixed in powerpc instead ? In that case, how ? Any idea ?


Looks like there are several other places where the problem occurs, so 
it has to be fixed in the powerpc headers instead.

Seems like RISC arch faced the same issue, and fixed it the following way:

/*
  * This is particularly ugly: it appears we can't actually get the 
definition
  * of task_struct here, but we need access to the CPU this task is 
running on.
  * Instead of using C we're using asm-offsets.h to get the current 
processor
  * ID.
  */
#define raw_smp_processor_id() (*((int*)((char*)get_current() + 
TASK_TI_CPU)))


Unless someone has a better idea, I'll fixed it that way.

Christophe


> 
>   fs/timerfd.c             | 2 +-
>   kernel/time/alarmtimer.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/timerfd.c b/fs/timerfd.c
> index d69ad801eb80..0fc01b241382 100644
> --- a/fs/timerfd.c
> +++ b/fs/timerfd.c
> @@ -9,12 +9,12 @@
>    *
>    */
>   
> -#include <linux/alarmtimer.h>
>   #include <linux/file.h>
>   #include <linux/poll.h>
>   #include <linux/init.h>
>   #include <linux/fs.h>
>   #include <linux/sched.h>
> +#include <linux/alarmtimer.h>
>   #include <linux/kernel.h>
>   #include <linux/slab.h>
>   #include <linux/list.h>
> diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
> index fa5de5e8de61..5fb75c9b3f06 100644
> --- a/kernel/time/alarmtimer.c
> +++ b/kernel/time/alarmtimer.c
> @@ -16,11 +16,11 @@
>    * published by the Free Software Foundation.
>    */
>   #include <linux/time.h>
> -#include <linux/hrtimer.h>
>   #include <linux/timerqueue.h>
>   #include <linux/rtc.h>
>   #include <linux/sched/signal.h>
>   #include <linux/sched/debug.h>
> +#include <linux/hrtimer.h>
>   #include <linux/alarmtimer.h>
>   #include <linux/mutex.h>
>   #include <linux/platform_device.h>
>