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: <9bff645b-4e00-ab0f-288a-a17f1e45ecdd@c-s.fr>
Date:   Tue, 25 Sep 2018 08:33:35 +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 25/09/2018 à 07:34, Christophe LEROY a écrit :
> 
> 
> 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.


Not easy. I'm stuck at the moment, because asm-offsets.h redefines a lot 
of things with the same name as in C, so when including asm-offsets.h in 
C it conflicts. Argh !. Need to fix all those duplicates ?

Christophe

> 
> 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>
>>