Re: [PATCH 2/3] ARM: Move thread_info into task_struct (v7 only)

[Keith Packard]

Ard Biesheuvel <ardb@xxxxxxxxxx> writes:

(sorry, missed a couple of comments, and also neglected to thank you for
your review!)

>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 24804f11302d..1c1ded500a2b 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -128,6 +128,7 @@ config ARM
>>         select RTC_LIB
>>         select SET_FS
>>         select SYS_SUPPORTS_APM_EMULATION
>> +       select THREAD_INFO_IN_TASK if CPU_V7
>
> CPU_V6K also supports this

I've only tested on V7; help getting testing for V6K so we could enable
that as well would be greatly appreciated.

>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>> index 415c3514573a..71a2ba4549d3 100644
>> --- a/arch/arm/Makefile
>> +++ b/arch/arm/Makefile
>> @@ -284,6 +284,14 @@ stack_protector_prepare: prepare0
>>         $(eval GCC_PLUGINS_CFLAGS += $(SSP_PLUGIN_CFLAGS))
>>  endif
>>
>> +ifdef CONFIG_SMP
>> +prepare: task_cpu_prepare
>> +
>> +PHONY += task_cpu_prepare
>> +task_cpu_prepare: prepare0
>> +       $(eval KBUILD_CFLAGS += -D_TSK_CPU=$(shell awk '{if ($$2 == "TSK_CPU") print $$3;}' include/generated/asm-offsets.h))
>> +endif
>> +
>>  all:   $(notdir $(KBUILD_IMAGE))
>>
>
> This is rather horrid, and removed again in the next patch. Can we
> omit it entirely?

Yeah, this did get rather ugly -- I wanted to use the task_struct in the
raw_smp_processor_id() macro, but discovered that I couldn't include
linux/sched.h in that file.

I found this lovely hack in the powerpc kernel code and just lifted it
from there. In terms of code changes, this one is "smaller" than
including the new per-cpu variable, "cpu_number", but in terms of
ugliness, it's hard to argue which is cleaner.

I was going for fewest logical changes in structure with this patch
instead of cleanest result to try and make it slightly easier to review.

Happy to squash these two patches together if you'd prefer; I initially
wrote the code with the cpu_number variable but then discovered that I
didn't need it if I used the cpu value in the task_struct...

-- 
-keith

Attachment: signature.asc
Description: PGP signature