[PATCH tip/core/rcu 0/2] Documentation updates

[PATCH tip/core/rcu 0/2] Documentation updates

[Paul E. McKenney]

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

[PATCH tip/core/rcu 1/2] rcutorture: Remove outdated config option description

[Paul E. McKenney]

From: SeongJae Park <sj38.park@gmail.com>

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 <sj38.park@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 Documentation/RCU/torture.txt | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/Documentation/RCU/torture.txt b/Documentation/RCU/torture.txt
index 118e7c176ce7..278f6a9383b6 100644
--- a/Documentation/RCU/torture.txt
+++ b/Documentation/RCU/torture.txt
@@ -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
 
-- 
2.5.2

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

[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 <paulmck@linux.vnet.ibm.com>
---
 .../RCU/Design/Requirements/Requirements.html      | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/RCU/Design/Requirements/Requirements.html b/Documentation/RCU/Design/Requirements/Requirements.html
index ece410f40436..a4d3838130e4 100644
--- a/Documentation/RCU/Design/Requirements/Requirements.html
+++ b/Documentation/RCU/Design/Requirements/Requirements.html
@@ -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>
 
-- 
2.5.2

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

[Peter Zijlstra]

On Mon, Aug 22, 2016 at 08:14:43AM -0700, Paul E. McKenney wrote:
> 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?

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

[Paul E. McKenney]

On Mon, Aug 22, 2016 at 06:25:53PM +0200, Peter Zijlstra wrote:
> On Mon, Aug 22, 2016 at 08:14:43AM -0700, Paul E. McKenney wrote:
> > 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?

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

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

[Eric Dumazet]

On Mon, Aug 22, 2016 at 10:34 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:

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

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

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

[Peter Zijlstra]

On Mon, Aug 22, 2016 at 11:48:53AM -0700, Eric Dumazet wrote:
> On Mon, Aug 22, 2016 at 10:34 AM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> 
> > 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));
> 
> 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.

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

[Steven Rostedt]

On Mon, 22 Aug 2016 20:56:09 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

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

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

[Paul E. McKenney]

On Mon, Aug 22, 2016 at 03:18:54PM -0400, Steven Rostedt wrote:
> On Mon, 22 Aug 2016 20:56:09 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > > 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.

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

> 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

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

[Geert Uytterhoeven]

Hi Paul,

On Mon, Aug 22, 2016 at 9:54 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Mon, Aug 22, 2016 at 03:18:54PM -0400, Steven Rostedt wrote:
>> On Mon, 22 Aug 2016 20:56:09 +0200
>> Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> > > 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 *))));

No, it's aligning to sizeof(void *) (4 on m68k), not __alignof__(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.

__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 -- geert@linux-m68k.org

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

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

[Paul E. McKenney]

On Mon, Aug 22, 2016 at 10:48:57PM +0200, Geert Uytterhoeven wrote:
> Hi Paul,
> 
> On Mon, Aug 22, 2016 at 9:54 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Mon, Aug 22, 2016 at 03:18:54PM -0400, Steven Rostedt wrote:
> >> On Mon, 22 Aug 2016 20:56:09 +0200
> >> Peter Zijlstra <peterz@infradead.org> wrote:
> >>
> >> > > 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 *))));
> 
> 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.

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

> 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

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

[Geert Uytterhoeven]

Hi Paul,

On Mon, Aug 22, 2016 at 11:16 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Mon, Aug 22, 2016 at 10:48:57PM +0200, Geert Uytterhoeven wrote:
>> On Mon, Aug 22, 2016 at 9:54 PM, Paul E. McKenney
>> <paulmck@linux.vnet.ibm.com> wrote:
>> > On Mon, Aug 22, 2016 at 03:18:54PM -0400, Steven Rostedt wrote:
>> >> On Mon, 22 Aug 2016 20:56:09 +0200
>> >> Peter Zijlstra <peterz@infradead.org> wrote:
>> >>
>> >> > > 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 *))));
>>
>> 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.
>
>> > #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?
>
>> 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?

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 -- geert@linux-m68k.org

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

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

[Paul E. McKenney]

On Tue, Aug 23, 2016 at 08:39:18AM +0200, Geert Uytterhoeven wrote:
> Hi Paul,
> 
> On Mon, Aug 22, 2016 at 11:16 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Mon, Aug 22, 2016 at 10:48:57PM +0200, Geert Uytterhoeven wrote:
> >> On Mon, Aug 22, 2016 at 9:54 PM, Paul E. McKenney
> >> <paulmck@linux.vnet.ibm.com> wrote:
> >> > On Mon, Aug 22, 2016 at 03:18:54PM -0400, Steven Rostedt wrote:
> >> >> On Mon, 22 Aug 2016 20:56:09 +0200
> >> >> Peter Zijlstra <peterz@infradead.org> wrote:
> >> >>
> >> >> > > 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 *))));
> >>
> >> 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.
> >
> >> > #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?
> >
> >> 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?
> 
> 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

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

[Geert Uytterhoeven]

Hi Paul,

On Tue, Aug 23, 2016 at 3:43 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Tue, Aug 23, 2016 at 08:39:18AM +0200, Geert Uytterhoeven wrote:
>> On Mon, Aug 22, 2016 at 11:16 PM, Paul E. McKenney
>> <paulmck@linux.vnet.ibm.com> wrote:
>> > On Mon, Aug 22, 2016 at 10:48:57PM +0200, Geert Uytterhoeven wrote:
>> >> On Mon, Aug 22, 2016 at 9:54 PM, Paul E. McKenney
>> >> <paulmck@linux.vnet.ibm.com> wrote:
>> >> > On Mon, Aug 22, 2016 at 03:18:54PM -0400, Steven Rostedt wrote:
>> >> >> On Mon, 22 Aug 2016 20:56:09 +0200
>> >> >> Peter Zijlstra <peterz@infradead.org> wrote:
>> >> >>
>> >> >> > > 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 *))));
>> >>
>> >> 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.
>> > 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));

Yes, I agree with that.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

[Paul E. McKenney]

On Tue, Aug 23, 2016 at 03:45:51PM +0200, Geert Uytterhoeven wrote:
> Hi Paul,
> 
> On Tue, Aug 23, 2016 at 3:43 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Tue, Aug 23, 2016 at 08:39:18AM +0200, Geert Uytterhoeven wrote:
> >> On Mon, Aug 22, 2016 at 11:16 PM, Paul E. McKenney
> >> <paulmck@linux.vnet.ibm.com> wrote:
> >> > On Mon, Aug 22, 2016 at 10:48:57PM +0200, Geert Uytterhoeven wrote:
> >> >> On Mon, Aug 22, 2016 at 9:54 PM, Paul E. McKenney
> >> >> <paulmck@linux.vnet.ibm.com> wrote:
> >> >> > On Mon, Aug 22, 2016 at 03:18:54PM -0400, Steven Rostedt wrote:
> >> >> >> On Mon, 22 Aug 2016 20:56:09 +0200
> >> >> >> Peter Zijlstra <peterz@infradead.org> wrote:
> >> >> >>
> >> >> >> > > 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 *))));
> >> >>
> >> >> 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.
> >> > 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));
> 
> Yes, I agree with that.

Very good, I have queued the following patch.

							Thanx, Paul

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

commit 89d39c83d193733ed5fff1c480cd42c9de1da404
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
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 <paulmck@linux.vnet.ibm.com>
    Cc: Geert Uytterhoeven <geert@linux-m68k.org>
    Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 3a8eec3ba1bd..673bcb3934a3 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -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);

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

[Geert Uytterhoeven]

On Tue, Aug 23, 2016 at 4:23 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> commit 89d39c83d193733ed5fff1c480cd42c9de1da404
> Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 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 <paulmck@linux.vnet.ibm.com>
>     Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>     Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

[Paul E. McKenney]

On Tue, Aug 23, 2016 at 04:30:47PM +0200, Geert Uytterhoeven wrote:
> On Tue, Aug 23, 2016 at 4:23 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > commit 89d39c83d193733ed5fff1c480cd42c9de1da404
> > Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 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 <paulmck@linux.vnet.ibm.com>
> >     Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> >     Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> 
> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

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

							Thanx, Paul

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

[Kirill A. Shutemov]

On Tue, Aug 23, 2016 at 07:23:26AM -0700, Paul E. McKenney wrote:
> On Tue, Aug 23, 2016 at 03:45:51PM +0200, Geert Uytterhoeven wrote:
> > Hi Paul,
> > 
> > On Tue, Aug 23, 2016 at 3:43 PM, Paul E. McKenney
> > <paulmck@linux.vnet.ibm.com> wrote:
> > > On Tue, Aug 23, 2016 at 08:39:18AM +0200, Geert Uytterhoeven wrote:
> > >> On Mon, Aug 22, 2016 at 11:16 PM, Paul E. McKenney
> > >> <paulmck@linux.vnet.ibm.com> wrote:
> > >> > On Mon, Aug 22, 2016 at 10:48:57PM +0200, Geert Uytterhoeven wrote:
> > >> >> On Mon, Aug 22, 2016 at 9:54 PM, Paul E. McKenney
> > >> >> <paulmck@linux.vnet.ibm.com> wrote:
> > >> >> > On Mon, Aug 22, 2016 at 03:18:54PM -0400, Steven Rostedt wrote:
> > >> >> >> On Mon, 22 Aug 2016 20:56:09 +0200
> > >> >> >> Peter Zijlstra <peterz@infradead.org> wrote:
> > >> >> >>
> > >> >> >> > > 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 *))));
> > >> >>
> > >> >> 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.
> > >> > 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));
> > 
> > Yes, I agree with that.
> 
> Very good, I have queued the following patch.
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> commit 89d39c83d193733ed5fff1c480cd42c9de1da404
> Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 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 <paulmck@linux.vnet.ibm.com>
>     Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>     Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Looks good to me.

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov