Le 03/10/2018 à 07:52, Nicholas Piggin a écrit :

On Wed, 3 Oct 2018 07:47:05 +0200
Christophe LEROY [off-list ref] wrote:

quoted

Le 03/10/2018 à 07:30, Nicholas Piggin a écrit :

quoted

On Mon,  1 Oct 2018 12:30:23 +0000 (UTC)
Christophe Leroy [off-list ref] wrote:
   

quoted

This patch activates CONFIG_THREAD_INFO_IN_TASK which
moves the thread_info into task_struct.

Moving thread_info into task_struct has the following advantages:
- It protects thread_info from corruption in the case of stack
overflows.
- Its address is harder to determine if stack addresses are
leaked, making a number of attacks more difficult.

This has the following consequences:
- thread_info is now located at the top of task_struct.

"top"... I got confused for a minute thinking high address and
wondering how you can change CURRENT_THREAD_INFO just to point
to current :)

Would 'beginning' be less confusing ?

Yes, good idea.

quoted

quoted

quoted

@@ -83,7 +83,13 @@ int is_cpu_dead(unsigned int cpu);
   /* 32-bit */
   extern int smp_hw_index[];
   
-#define raw_smp_processor_id()	(current_thread_info()->cpu)
+/*
+ * 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 task_struct we're using _TASK_CPU which is extracted from
+ * asm-offsets.h by kbuild to get the current processor ID.
+ */
+#define raw_smp_processor_id()		(*(unsigned int*)((void*)current + _TASK_CPU))

This is clever but yes ugly. Can't you include asm-offsets.h? riscv
seems to.

riscv has a clean asm-offsets.h . Our's defines constant with the same
name as those defined in other headers which are included in C files. So
including asm-offsets in C files does create conflicts like:

./include/generated/asm-offsets.h:71:0: warning: "TASK_SIZE" redefined
   #define TASK_SIZE -2147483648 /* TASK_SIZE */
./arch/powerpc/include/asm/processor.h:95:0: note: this is the location
of the previous definition
   #define TASK_SIZE (CONFIG_TASK_SIZE)

./include/generated/asm-offsets.h:98:0: warning: "NSEC_PER_SEC" redefined
   #define NSEC_PER_SEC 1000000000 /* NSEC_PER_SEC */
./include/linux/time64.h:36:0: note: this is the location of the
previous definition
   #define NSEC_PER_SEC 1000000000L

./arch/powerpc/include/asm/nohash/32/pgtable.h:34:0: warning:
"PGD_TABLE_SIZE" redefined
   #define PGD_TABLE_SIZE (sizeof(pgd_t) << PGD_INDEX_SIZE)
./include/generated/asm-offsets.h:101:0: note: this is the location of
the previous definition
   #define PGD_TABLE_SIZE 256 /* PGD_TABLE_SIZE */

...

Okay.

quoted

In v2, I had a patch to fix those redundancies
(https://patchwork.ozlabs.org/patch/974363/) but I found it unconvenient.

Because of merge conflicts, or you did not like the new names?

Both, because of the amount of changes it implies, and also because of 
the new names. I find it quite convenient to be able to use same names 
both in C and ASM. And I didn't want my serie to imply big-bangs in 
unrelated or not directly related topics.

Christophe

Thanks,
Nick