Documentation updates

PATCH In review Paul E. McKenney 6 participants 18 messages Aug 22, 2016 – Aug 24, 2016

Subsystems (2)

Documentation RCU

lore

Share & tools

lore

Paul E. McKenney

author

2016-08-22 15:14 UTC #0 lore raw

cover Documentation updates

Hello!

This series provides a couple of documentation updates:

1. Remove obsolete CONFIG_RCU_TORTURE_TEST_RUNNABLE description,
courtesy of SeongJae Park.

2. Record the reason for two-byte (rather than four- or eight-byte)
alignment for the rcu_head structure.

Thanx, Paul

------------------------------------------------------------------------

 Design/Requirements/Requirements.html |   22 ++++++++++++++++++++++
 torture.txt                           |   15 ---------------
 2 files changed, 22 insertions(+), 15 deletions(-)

Paul E. McKenney

author

2016-08-22 15:15 UTC #1 lore raw

PATCH 1/2 rcutorture: Remove outdated config option description

From: SeongJae Park <[email protected]>

CONFIG_RCU_TORTURE_TEST_RUNNABLE has removed by commit 4e9a073f60367 ("torture: Remove CONFIG_RCU_TORTURE_TEST_RUNNABLE, simplify code") entirely but the document has not updated. This commit updates the document to remove the description for the config option and adding a description for the alternative module parameter.

Signed-off-by SeongJae Park <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

---
 Documentation/RCU/torture.txt | 15 ---------------
 1 file changed, 15 deletions(-)

Documentation/RCU/torture.txt +0 ~~-15~~

index 118e7c176ce7..278f6a9383b6 100644
@@ -10,21 +10,6 @@ status messages via printk(), which can be examined via the dmesg
 command (perhaps grepping for "torture").  The test is started
 when the module is loaded, and stops when the module is unloaded.
 
-CONFIG_RCU_TORTURE_TEST_RUNNABLE
-
-It is also possible to specify CONFIG_RCU_TORTURE_TEST=y, which will
-result in the tests being loaded into the base kernel.  In this case,
-the CONFIG_RCU_TORTURE_TEST_RUNNABLE config option is used to specify
-whether the RCU torture tests are to be started immediately during
-boot or whether the /proc/sys/kernel/rcutorture_runnable file is used
-to enable them.  This /proc file can be used to repeatedly pause and
-restart the tests, regardless of the initial state specified by the
-CONFIG_RCU_TORTURE_TEST_RUNNABLE config option.
-
-You will normally -not- want to start the RCU torture tests during boot
-(and thus the default is CONFIG_RCU_TORTURE_TEST_RUNNABLE=n), but doing
-this can sometimes be useful in finding boot-time bugs.
-
 
 MODULE PARAMETERS

Signature · 1 line

2.5.2

Paul E. McKenney

author

2016-08-22 15:15 UTC #2 lore raw

PATCH 2/2 documentation: Record reason for rcu_head two-byte alignment

In reply to Paul E. McKenney

The __call_rcu() assertion that checks only the bottom bit of the rcu_head pointer is a bit counter-intuitive in these days of ubiquitous 64-bit systems. This commit therefore records the reason for this odd alignment check, namely that m68k guarantees only two-byte alignment despite being a 32-bit architectures.

Signed-off-by Paul E. McKenney <[email protected]>

---
 .../RCU/Design/Requirements/Requirements.html      | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Documentation/RCU/Design/Requirements/Requirements.html +22 -0

index ece410f40436..a4d3838130e4 100644
@@ -2493,6 +2493,28 @@ or some future &ldquo;lazy&rdquo;
 variant of <tt>call_rcu()</tt> that might one day be created for
 energy-efficiency purposes.
 
+<p>
+That said, there are limits.
+RCU requires that the <tt>rcu_head</tt> structure be aligned to a
+two-byte boundary, and passing a misaligned <tt>rcu_head</tt>
+structure to one of the <tt>call_rcu()</tt> family of functions
+will result in a splat.
+It is therefore necessary to exercise caution when packing
+structures containing fields of type <tt>rcu_head</tt>.
+Why not a four-byte or even eight-byte alignment requirement?
+Because the m68k architecture provides only two-byte alignment,
+and thus acts as alignment's least common denominator.
+
+<p>
+The reason for reserving the bottom bit of pointers to
+<tt>rcu_head</tt> structures is to leave the door open to
+&ldquo;lazy&rdquo; callbacks whose invocations can safely be deferred.
+Deferring invocation could potentially have energy-efficiency
+benefits, but only if the rate of non-lazy callbacks decreases
+significantly for some important workload.
+In the meantime, reserving the bottom bit keeps this option open
+in case it one day becomes useful.
+
 <h3><a name="Performance, Scalability, Response Time, and Reliability">
 Performance, Scalability, Response Time, and Reliability</a></h3>

Signature · 1 line

2.5.2

Peter Zijlstra

2016-08-22 16:26 UTC #3 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 5 lines · Paul E. McKenney

The __call_rcu() assertion that checks only the bottom bit of the rcu_head pointer is a bit counter-intuitive in these days of ubiquitous 64-bit systems. This commit therefore records the reason for this odd alignment check, namely that m68k guarantees only two-byte alignment despite being a 32-bit architectures.

Would not something like:

#ifdef CONFIG_M68K
	/*
	 * m68k is weird and doesn't have naturally aligned types.
	 */
	WARN_ON_ONCE((unsigned long)head & 1);
#else
	WARN_ON_ONCE((unsigned long)head & (sizeof(unsigned long) - 1));

#endif

Be better?

Paul E. McKenney

author

2016-08-22 17:34 UTC #4 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 16 lines · Peter Zijlstra

Quoted 5 lines · Paul E. McKenney
The __call_rcu() assertion that checks only the bottom bit of the rcu_head pointer is a bit counter-intuitive in these days of ubiquitous 64-bit systems. This commit therefore records the reason for this odd alignment check, namely that m68k guarantees only two-byte alignment despite being a 32-bit architectures.
Would not something like:
#ifdef CONFIG_M68K
	/*
	 * m68k is weird and doesn't have naturally aligned types.
	 */
	WARN_ON_ONCE((unsigned long)head & 1);
#else
	WARN_ON_ONCE((unsigned long)head & (sizeof(unsigned long) - 1));

Quoted 3 lines

#endif
Be better?

That does have much to say for itself, though I would prefer sizeof(void *) to sizeof(unsigned long). But would it make sense to define a mask on a per-architecture basis, with the default being (sizeof(void *) - 1)? Then maybe an IMPROPERLY_ALIGNED_POINTER():

	#ifndef CONFIG_ARCH_POINTER_ALIGNMENT
	#define CONFIG_ARCH_POINTER_ALIGNMENT (sizeof(void *) - 1)
	#endif

	#define IMPROPERLY_ALIGNED_POINTER(p) \
		((p) & CONFIG_ARCH_POINTER_ALIGNMENT)

m68k would define ARCH_POINTER_ALIGNMENT to 1, and all other arches would leave it undefined.

Then __call_rcu() could to this:

	WARN_ON_ONCE(IMPROPERLY_ALIGNED_POINTER(head));

Seem reasonable?

Thanx, Paul

Eric Dumazet

2016-08-22 18:48 UTC #5 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 12 lines · Paul E. McKenney

That does have much to say for itself, though I would prefer sizeof(void *) to sizeof(unsigned long). But would it make sense to define a mask on a per-architecture basis, with the default being (sizeof(void *) - 1)? Then maybe an IMPROPERLY_ALIGNED_POINTER():
        #ifndef CONFIG_ARCH_POINTER_ALIGNMENT
        #define CONFIG_ARCH_POINTER_ALIGNMENT (sizeof(void *) - 1)
        #endif

        #define IMPROPERLY_ALIGNED_POINTER(p) \
                ((p) & CONFIG_ARCH_POINTER_ALIGNMENT)

Quoted 6 lines

m68k would define ARCH_POINTER_ALIGNMENT to 1, and all other arches would leave it undefined.
Then __call_rcu() could to this:
        WARN_ON_ONCE(IMPROPERLY_ALIGNED_POINTER(head));

Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?

Peter Zijlstra

2016-08-22 18:56 UTC #6 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 18 lines · Eric Dumazet

Quoted 15 lines · Paul E. McKenney
That does have much to say for itself, though I would prefer sizeof(void *) to sizeof(unsigned long). But would it make sense to define a mask on a per-architecture basis, with the default being (sizeof(void *) - 1)? Then maybe an IMPROPERLY_ALIGNED_POINTER():
        #ifndef CONFIG_ARCH_POINTER_ALIGNMENT
        #define CONFIG_ARCH_POINTER_ALIGNMENT (sizeof(void *) - 1)
        #endif

        #define IMPROPERLY_ALIGNED_POINTER(p) \
                ((p) & CONFIG_ARCH_POINTER_ALIGNMENT)
m68k would define ARCH_POINTER_ALIGNMENT to 1, and all other arches would leave it undefined.

Quoted 6 lines

Quoted 3 lines
Then __call_rcu() could to this:
        WARN_ON_ONCE(IMPROPERLY_ALIGNED_POINTER(head));
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?

Yes, but that 'hides' the m68k funny, while doing an explicit #ifdef has documentation value... but I don't care too deeply.

Steven Rostedt

reviewer

2016-08-22 19:19 UTC #7 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 5 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:

Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?
Yes, but that 'hides' the m68k funny, while doing an explicit #ifdef has documentation value... but I don't care too deeply.

I'd recommend keeping the #ifdef, and then if another architecture comes along that is as weird as m68k, we can use the generic __alignof__(void *). Maybe even add that in the comment, so when/if that arch is created, people will know how to fix it more generically.

-- Steve

Paul E. McKenney

author

2016-08-22 19:54 UTC #8 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 5 lines · Steven Rostedt

Quoted 2 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?

Hmmm... Does __alignof__(void *) give two-byte alignment on m68k, allowing something like this? Heh!!! It is already there. ;-)

struct callback_head {
	struct callback_head *next;
	void (*func)(struct callback_head *head);
} __attribute__((aligned(sizeof(void *))));

#define rcu_head callback_head

If so, that does sound quite attractive! Might need the WARN_ON() anyway, to flag wild pointers if nothing else.

Adding Geert on CC for his thoughts.

Quoted 2 lines

Quoted 2 lines
Yes, but that 'hides' the m68k funny, while doing an explicit #ifdef has documentation value... but I don't care too deeply.

Well, if I need the WARN_ON() anyway, perhaps we get both.

Quoted 4 lines

I'd recommend keeping the #ifdef, and then if another architecture comes along that is as weird as m68k, we can use the generic __alignof__(void *). Maybe even add that in the comment, so when/if that arch is created, people will know how to fix it more generically.

Maybe __call_rcu() can do something like this?

	WARN_ON_ONCE((unsigned long)head & (__alignof__(struct rcu_head) - 1));

Except that RCU needs at least two-byte alignment, so something like this?

	WARN_ON_ONCE((unsigned long)head &
		     ((__alignof__(struct rcu_head) - 1) | 0x1));

That way, some future architecture that doesn't believe in any alignment at all will be properly informed of RCU's needs in this area.

Thanx, Paul

Geert Uytterhoeven

2016-08-22 20:49 UTC #9 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Hi Paul,

Quoted 7 lines · Paul E. McKenney

Quoted 5 lines · Steven Rostedt
Quoted 2 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?

Quoted 7 lines

Hmmm... Does __alignof__(void *) give two-byte alignment on m68k, allowing something like this? Heh!!! It is already there. ;-)
struct callback_head {
        struct callback_head *next;
        void (*func)(struct callback_head *head);
} __attribute__((aligned(sizeof(void *))));

No, it's aligning to sizeof(void *) (4 on m68k), not __alignof__(void *).

Quoted 6 lines

#define rcu_head callback_head
If so, that does sound quite attractive! Might need the WARN_ON() anyway, to flag wild pointers if nothing else.
Adding Geert on CC for his thoughts.

__alignof__(void *) is indeed 2 on m68k, and h8300.

Note that it is 1 on crisv32!

It's 4 or 8 on anything else I have a cross-compiler for.

$ cat a.c
unsigned x = __alignof__(void *);
$ for i in /opt/cross/*/*/bin/*gcc; do echo +++ $i +++; $i -c -S a.c;
cat a.s; done | less

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Paul E. McKenney

author

2016-08-22 21:16 UTC #10 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 14 lines · Geert Uytterhoeven

Hi Paul,
Quoted 10 lines · Paul E. McKenney
Quoted 5 lines · Steven Rostedt
Quoted 2 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?
Hmmm... Does __alignof__(void *) give two-byte alignment on m68k, allowing something like this? Heh!!! It is already there. ;-)

Quoted 6 lines

Quoted 4 lines
struct callback_head {
        struct callback_head *next;
        void (*func)(struct callback_head *head);
} __attribute__((aligned(sizeof(void *))));
No, it's aligning to sizeof(void *) (4 on m68k), not __alignof__(void *).

Right you are. Commit 720abae3d68ae from Kirill A. Shutemov in November 2015.

Given that you haven't complained, I am guessing that this works for you. If so, I can make the __call_rcu() WARN_ON() more strict.

Quoted 10 lines

Quoted 6 lines
#define rcu_head callback_head
If so, that does sound quite attractive! Might need the WARN_ON() anyway, to flag wild pointers if nothing else.
Adding Geert on CC for his thoughts.
__alignof__(void *) is indeed 2 on m68k, and h8300.
Note that it is 1 on crisv32!

Gah... ((__alignof__(void *) + 1) & ~0x1), eh?

Quoted 6 lines

It's 4 or 8 on anything else I have a cross-compiler for.

$ cat a.c
unsigned x = __alignof__(void *);
$ for i in /opt/cross/*/*/bin/*gcc; do echo +++ $i +++; $i -c -S a.c;
cat a.s; done | less

Thank you for checking!

Again, does the current state work for you?

Thanx, Paul

Geert Uytterhoeven

2016-08-23 06:39 UTC #11 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Hi Paul,

Quoted 41 lines · Paul E. McKenney

Quoted 12 lines · Geert Uytterhoeven
Quoted 10 lines · Paul E. McKenney
Quoted 5 lines · Steven Rostedt
Quoted 2 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?
Hmmm... Does __alignof__(void *) give two-byte alignment on m68k, allowing something like this? Heh!!! It is already there. ;-)
Quoted 6 lines
Quoted 4 lines
struct callback_head {
        struct callback_head *next;
        void (*func)(struct callback_head *head);
} __attribute__((aligned(sizeof(void *))));
No, it's aligning to sizeof(void *) (4 on m68k), not __alignof__(void *).
Right you are. Commit 720abae3d68ae from Kirill A. Shutemov in November 2015.
Given that you haven't complained, I am guessing that this works for you. If so, I can make the __call_rcu() WARN_ON() more strict.
Quoted 10 lines
Quoted 6 lines
#define rcu_head callback_head
If so, that does sound quite attractive! Might need the WARN_ON() anyway, to flag wild pointers if nothing else.
Adding Geert on CC for his thoughts.
__alignof__(void *) is indeed 2 on m68k, and h8300.
Note that it is 1 on crisv32!
Gah... ((__alignof__(void *) + 1) & ~0x1), eh?
Quoted 2 lines
It's 4 or 8 on anything else I have a cross-compiler for.

Quoted 8 lines

Quoted 4 lines
$ cat a.c
unsigned x = __alignof__(void *);
$ for i in /opt/cross/*/*/bin/*gcc; do echo +++ $i +++; $i -c -S a.c;
cat a.s; done | less
Thank you for checking!
Again, does the current state work for you?

Yes it does. See also your commit 1146edcbef378922 ("rcu: Loosen __call_rcu()'s rcu_head alignment constraint").

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Paul E. McKenney

author

2016-08-23 13:43 UTC #12 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 50 lines · Geert Uytterhoeven

Hi Paul,
Quoted 46 lines · Paul E. McKenney
Quoted 12 lines · Geert Uytterhoeven
Quoted 10 lines · Paul E. McKenney
Quoted 5 lines · Steven Rostedt
Quoted 2 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?
Hmmm... Does __alignof__(void *) give two-byte alignment on m68k, allowing something like this? Heh!!! It is already there. ;-)
Quoted 6 lines
Quoted 4 lines
struct callback_head {
        struct callback_head *next;
        void (*func)(struct callback_head *head);
} __attribute__((aligned(sizeof(void *))));
No, it's aligning to sizeof(void *) (4 on m68k), not __alignof__(void *).
Right you are. Commit 720abae3d68ae from Kirill A. Shutemov in November 2015.
Given that you haven't complained, I am guessing that this works for you. If so, I can make the __call_rcu() WARN_ON() more strict.
Quoted 10 lines
Quoted 6 lines
#define rcu_head callback_head
If so, that does sound quite attractive! Might need the WARN_ON() anyway, to flag wild pointers if nothing else.
Adding Geert on CC for his thoughts.
__alignof__(void *) is indeed 2 on m68k, and h8300.
Note that it is 1 on crisv32!
Gah... ((__alignof__(void *) + 1) & ~0x1), eh?
Quoted 6 lines
It's 4 or 8 on anything else I have a cross-compiler for.
$ cat a.c
unsigned x = __alignof__(void *);
$ for i in /opt/cross/*/*/bin/*gcc; do echo +++ $i +++; $i -c -S a.c;
cat a.s; done | less

Quoted 6 lines

Quoted 3 lines
Thank you for checking!
Again, does the current state work for you?
Yes it does. See also your commit 1146edcbef378922 ("rcu: Loosen __call_rcu()'s rcu_head alignment constraint").

Understood!

But given that all architectures now provide at least four-byte alignment for the rcu_head structure, isn't it now OK for me to tighten up __call_rcu()'s check, for example, to this?

	WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1));

							Thanx, Paul

Geert Uytterhoeven

2016-08-23 14:02 UTC #13 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Hi Paul,

Quoted 33 lines · Paul E. McKenney

Quoted 25 lines · Geert Uytterhoeven
Quoted 23 lines · Paul E. McKenney
Quoted 12 lines · Geert Uytterhoeven
Quoted 10 lines · Paul E. McKenney
Quoted 5 lines · Steven Rostedt
Quoted 2 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?
Hmmm... Does __alignof__(void *) give two-byte alignment on m68k, allowing something like this? Heh!!! It is already there. ;-)
Quoted 6 lines
Quoted 4 lines
struct callback_head {
        struct callback_head *next;
        void (*func)(struct callback_head *head);
} __attribute__((aligned(sizeof(void *))));
No, it's aligning to sizeof(void *) (4 on m68k), not __alignof__(void *).
Right you are. Commit 720abae3d68ae from Kirill A. Shutemov in November 2015.
Quoted 6 lines
Quoted 3 lines
Given that you haven't complained, I am guessing that this works for you. If so, I can make the __call_rcu() WARN_ON() more strict. Again, does the current state work for you?
Yes it does. See also your commit 1146edcbef378922 ("rcu: Loosen __call_rcu()'s rcu_head alignment constraint").

Quoted 7 lines

Understood!
But given that all architectures now provide at least four-byte alignment for the rcu_head structure, isn't it now OK for me to tighten up __call_rcu()'s check, for example, to this?
        WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1));

Yes, I agree with that.

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Paul E. McKenney

author

2016-08-23 18:42 UTC #14 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 39 lines · Geert Uytterhoeven

Hi Paul,
Quoted 35 lines · Paul E. McKenney
Quoted 25 lines · Geert Uytterhoeven
Quoted 23 lines · Paul E. McKenney
Quoted 12 lines · Geert Uytterhoeven
Quoted 10 lines · Paul E. McKenney
Quoted 5 lines · Steven Rostedt
Quoted 2 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?
Hmmm... Does __alignof__(void *) give two-byte alignment on m68k, allowing something like this? Heh!!! It is already there. ;-)
Quoted 6 lines
Quoted 4 lines
struct callback_head {
        struct callback_head *next;
        void (*func)(struct callback_head *head);
} __attribute__((aligned(sizeof(void *))));
No, it's aligning to sizeof(void *) (4 on m68k), not __alignof__(void *).
Right you are. Commit 720abae3d68ae from Kirill A. Shutemov in November 2015.
Quoted 6 lines
Quoted 3 lines
Given that you haven't complained, I am guessing that this works for you. If so, I can make the __call_rcu() WARN_ON() more strict. Again, does the current state work for you?
Yes it does. See also your commit 1146edcbef378922 ("rcu: Loosen __call_rcu()'s rcu_head alignment constraint").
Understood!

Quoted 7 lines

Quoted 5 lines
But given that all architectures now provide at least four-byte alignment for the rcu_head structure, isn't it now OK for me to tighten up __call_rcu()'s check, for example, to this?
        WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1));
Yes, I agree with that.

Very good, I have queued the following patch.

Thanx, Paul

------------------------------------------------------------------------

commit 89d39c83d193733ed5fff1c480cd42c9de1da404

Author: Paul E. McKenney <[email protected]>
Date:   Tue Aug 23 06:51:47 2016 -0700

rcu: Tighted up __call_rcu() rcu_head alignment check

Commit 720abae3d68ae ("rcu: force alignment on struct callback_head/rcu_head") forced the rcu_head (AKA callback_head) structure's alignment to pointer size, that is, to 4-byte boundaries on 32-bit systems and to 8-byte boundaries on 64-bit systems. This commit therefore checks for this same alignment in __call_rcu(), which used to check for two-byte alignment.

Signed-off-by Paul E. McKenney <[email protected]>

Cc Geert Uytterhoeven <[email protected]>

Cc Kirill A. Shutemov <[email protected]>

kernel/rcu/tree.c +3 -1

index 3a8eec3ba1bd..673bcb3934a3 100644
@@ -3122,7 +3122,9 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func,
 	unsigned long flags;
 	struct rcu_data *rdp;
 
-	WARN_ON_ONCE((unsigned long)head & 0x1); /* Misaligned rcu_head! */
+	/* Misaligned rcu_head! */
+	WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1));
+
 	if (debug_rcu_head_queue(head)) {
 		/* Probable double call_rcu(), so leak the callback. */
 		WRITE_ONCE(head->func, rcu_leak_callback);

Geert Uytterhoeven

Tested-by

2016-08-23 14:30 UTC #15 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 13 lines · Paul E. McKenney

commit 89d39c83d193733ed5fff1c480cd42c9de1da404
Author: Paul E. McKenney <[email protected]>
Date:   Tue Aug 23 06:51:47 2016 -0700
rcu: Tighted up __call_rcu() rcu_head alignment check
Commit 720abae3d68ae ("rcu: force alignment on struct callback_head/rcu_head") forced the rcu_head (AKA callback_head) structure's alignment to pointer size, that is, to 4-byte boundaries on 32-bit systems and to 8-byte boundaries on 64-bit systems. This commit therefore checks for this same alignment in __call_rcu(), which used to check for two-byte alignment.

Quoted 3 lines

Signed-off-by Paul E. McKenney <[email protected]>
Cc Geert Uytterhoeven <[email protected]>
Cc Kirill A. Shutemov <[email protected]>

Tested-by Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Paul E. McKenney

author

2016-08-23 19:52 UTC #16 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

Quoted 15 lines · Geert Uytterhoeven

Quoted 13 lines · Paul E. McKenney
commit 89d39c83d193733ed5fff1c480cd42c9de1da404
Author: Paul E. McKenney <[email protected]>
Date:   Tue Aug 23 06:51:47 2016 -0700
rcu: Tighted up __call_rcu() rcu_head alignment check
Commit 720abae3d68ae ("rcu: force alignment on struct callback_head/rcu_head") forced the rcu_head (AKA callback_head) structure's alignment to pointer size, that is, to 4-byte boundaries on 32-bit systems and to 8-byte boundaries on 64-bit systems. This commit therefore checks for this same alignment in __call_rcu(), which used to check for two-byte alignment.

Quoted 5 lines

Quoted 3 lines
Signed-off-by Paul E. McKenney <[email protected]>
Cc Geert Uytterhoeven <[email protected]>
Cc Kirill A. Shutemov <[email protected]>
Tested-by Geert Uytterhoeven <[email protected]>

Thank you! I have upgraded your Cc to Tested-by. ;-)

Thanx, Paul

Kirill A. Shutemov

Acked-by

2016-08-24 18:05 UTC #17 lore raw

Re: [PATCH tip/core/rcu 2/2] documentation: Record reason for rcu_head two-byte alignment

In reply to Paul E. McKenney

Quoted 67 lines · Paul E. McKenney

Quoted 39 lines · Geert Uytterhoeven
Hi Paul,
Quoted 35 lines · Paul E. McKenney
Quoted 25 lines · Geert Uytterhoeven
Quoted 23 lines · Paul E. McKenney
Quoted 12 lines · Geert Uytterhoeven
Quoted 10 lines · Paul E. McKenney
Quoted 5 lines · Steven Rostedt
Quoted 2 lines · On Mon, 22 Aug 2016 20:56:09 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines
Don't we have __alignof__(void *) to avoid #ifdef CONFIG_M68K and other new macros ?
Hmmm... Does __alignof__(void *) give two-byte alignment on m68k, allowing something like this? Heh!!! It is already there. ;-)
Quoted 6 lines
Quoted 4 lines
struct callback_head {
        struct callback_head *next;
        void (*func)(struct callback_head *head);
} __attribute__((aligned(sizeof(void *))));
No, it's aligning to sizeof(void *) (4 on m68k), not __alignof__(void *).
Right you are. Commit 720abae3d68ae from Kirill A. Shutemov in November 2015.
Quoted 6 lines
Quoted 3 lines
Given that you haven't complained, I am guessing that this works for you. If so, I can make the __call_rcu() WARN_ON() more strict. Again, does the current state work for you?
Yes it does. See also your commit 1146edcbef378922 ("rcu: Loosen __call_rcu()'s rcu_head alignment constraint").
Understood!
Quoted 7 lines
Quoted 5 lines
But given that all architectures now provide at least four-byte alignment for the rcu_head structure, isn't it now OK for me to tighten up __call_rcu()'s check, for example, to this?
        WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1));
Yes, I agree with that.
Very good, I have queued the following patch.
Thanx, Paul
------------------------------------------------------------------------
commit 89d39c83d193733ed5fff1c480cd42c9de1da404
Author: Paul E. McKenney <[email protected]>
Date:   Tue Aug 23 06:51:47 2016 -0700
rcu: Tighted up __call_rcu() rcu_head alignment check
Commit 720abae3d68ae ("rcu: force alignment on struct callback_head/rcu_head") forced the rcu_head (AKA callback_head) structure's alignment to pointer size, that is, to 4-byte boundaries on 32-bit systems and to 8-byte boundaries on 64-bit systems. This commit therefore checks for this same alignment in __call_rcu(), which used to check for two-byte alignment.

Quoted 3 lines

Signed-off-by Paul E. McKenney <[email protected]>
Cc Geert Uytterhoeven <[email protected]>
Cc Kirill A. Shutemov <[email protected]>

Looks good to me.

Acked-by Kirill A. Shutemov <[email protected]>

Signature · 1 line

 Kirill A. Shutemov