Re: rcu alignment warning tripping on m68k

2014-06-09 Thread Greg Ungerer
Hi Paul,

On 07/06/14 04:46, Paul E. McKenney wrote:
> On Fri, May 30, 2014 at 11:29:41AM +1000, Greg Ungerer wrote:
>> On 29/05/14 23:11, One Thousand Gnomes wrote:
>>> On Thu, 29 May 2014 12:08:32 +1000
>>> Greg Ungerer  wrote:
>>>
 Hi All,

 Inside kernel/rcy/tree.c in __call_rcu() it does an alignment check on
 the head pointer passed in. This trips on m68k systems, because they only
 need alignment of 32bit quantities to 16bit boundaries.
>>>
>>> __alignof perhaps ?
>>
>> That might do. Change then becomes something like:
>>
>> --- a/kernel/rcu/tree.c
>> +++ b/kernel/rcu/tree.c
>> @@ -2467,7 +2467,7 @@ __call_rcu(struct rcu_head *head, void (*func)(struct 
>> rcu_
>> unsigned long flags;
>> struct rcu_data *rdp;
>>
>> -   WARN_ON_ONCE((unsigned long)head & 0x3); /* Misaligned rcu_head! */
>> +   WARN_ON_ONCE((unsigned long)head & (__alignof__(head) - 1)); /* 
>> Misaligned rcu_head! */
> 
> Hmmm...  The purpose of the check is to reserve the low-order bits to
> allow RCU to classify callbacks as being time-critical or not.  RCU
> can probably live with a single bit, but if there is some architecture
> out there that simply refuses to do alignment, I need to know about it.

This change was prompted by this check tripping, so the alignment
issue is certainly real for m68k.

Regards
Greg


> (See "git show 0bb7b59d6e2b8" for more info.)
> 
> So how about this instead?
> 
>  -   WARN_ON_ONCE((unsigned long)head & 0x1); /* Misaligned rcu_head! */
> 
> (Trying to remember if I have seen Linux kernel code that uses both
> the lower bits...)
> 
>   Thanx, Paul
> 
>> if (debug_rcu_head_queue(head)) {
>> /* Probable double call_rcu(), so leak the callback. */
>> ACCESS_ONCE(head->func) = rcu_leak_callback;
>>
>> Thanks
>> Greg
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: rcu alignment warning tripping on m68k

2014-06-09 Thread Paul E. McKenney
On Sat, Jun 07, 2014 at 03:17:24PM +0200, Mikael Pettersson wrote:
> Paul E. McKenney writes:
>  > On Fri, May 30, 2014 at 11:29:41AM +1000, Greg Ungerer wrote:
>  > > On 29/05/14 23:11, One Thousand Gnomes wrote:
>  > > > On Thu, 29 May 2014 12:08:32 +1000
>  > > > Greg Ungerer  wrote:
>  > > > 
>  > > >> Hi All,
>  > > >>
>  > > >> Inside kernel/rcy/tree.c in __call_rcu() it does an alignment check on
>  > > >> the head pointer passed in. This trips on m68k systems, because they 
> only
>  > > >> need alignment of 32bit quantities to 16bit boundaries.
>  > > > 
>  > > > __alignof perhaps ?
>  > > 
>  > > That might do. Change then becomes something like:
>  > > 
>  > > --- a/kernel/rcu/tree.c
>  > > +++ b/kernel/rcu/tree.c
>  > > @@ -2467,7 +2467,7 @@ __call_rcu(struct rcu_head *head, void 
> (*func)(struct rcu_
>  > > unsigned long flags;
>  > > struct rcu_data *rdp;
>  > > 
>  > > -   WARN_ON_ONCE((unsigned long)head & 0x3); /* Misaligned rcu_head! 
> */
>  > > +   WARN_ON_ONCE((unsigned long)head & (__alignof__(head) - 1)); /* 
> Misaligned rcu_head! */
>  > 
>  > Hmmm...  The purpose of the check is to reserve the low-order bits to
>  > allow RCU to classify callbacks as being time-critical or not.  RCU
>  > can probably live with a single bit, but if there is some architecture
>  > out there that simply refuses to do alignment, I need to know about it.
>  > 
>  > (See "git show 0bb7b59d6e2b8" for more info.)
>  > 
>  > So how about this instead?
>  > 
>  >  -   WARN_ON_ONCE((unsigned long)head & 0x1); /* Misaligned rcu_head! 
> */
>  > 
>  > (Trying to remember if I have seen Linux kernel code that uses both
>  > the lower bits...)
> 
> As stated above, m68k-linux aligns to 16-bit boundaries by default, so you'd
> get one bit but not necessarily more.  If you want more free low bits, why
> not attach an explicit attribute aligned to the rcu_head type declaration?

One bit should do it for the time being, but yes, if I ever need two bits,
your suggestion of explicitly aligning the rcu_head type declaration
sounds like a very good one.

Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: rcu alignment warning tripping on m68k

2014-06-07 Thread Mikael Pettersson
Paul E. McKenney writes:
 > On Fri, May 30, 2014 at 11:29:41AM +1000, Greg Ungerer wrote:
 > > On 29/05/14 23:11, One Thousand Gnomes wrote:
 > > > On Thu, 29 May 2014 12:08:32 +1000
 > > > Greg Ungerer  wrote:
 > > > 
 > > >> Hi All,
 > > >>
 > > >> Inside kernel/rcy/tree.c in __call_rcu() it does an alignment check on
 > > >> the head pointer passed in. This trips on m68k systems, because they 
 > > >> only
 > > >> need alignment of 32bit quantities to 16bit boundaries.
 > > > 
 > > > __alignof perhaps ?
 > > 
 > > That might do. Change then becomes something like:
 > > 
 > > --- a/kernel/rcu/tree.c
 > > +++ b/kernel/rcu/tree.c
 > > @@ -2467,7 +2467,7 @@ __call_rcu(struct rcu_head *head, void 
 > > (*func)(struct rcu_
 > > unsigned long flags;
 > > struct rcu_data *rdp;
 > > 
 > > -   WARN_ON_ONCE((unsigned long)head & 0x3); /* Misaligned rcu_head! */
 > > +   WARN_ON_ONCE((unsigned long)head & (__alignof__(head) - 1)); /* 
 > > Misaligned rcu_head! */
 > 
 > Hmmm...  The purpose of the check is to reserve the low-order bits to
 > allow RCU to classify callbacks as being time-critical or not.  RCU
 > can probably live with a single bit, but if there is some architecture
 > out there that simply refuses to do alignment, I need to know about it.
 > 
 > (See "git show 0bb7b59d6e2b8" for more info.)
 > 
 > So how about this instead?
 > 
 >  -   WARN_ON_ONCE((unsigned long)head & 0x1); /* Misaligned rcu_head! */
 > 
 > (Trying to remember if I have seen Linux kernel code that uses both
 > the lower bits...)

As stated above, m68k-linux aligns to 16-bit boundaries by default, so you'd
get one bit but not necessarily more.  If you want more free low bits, why
not attach an explicit attribute aligned to the rcu_head type declaration?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: rcu alignment warning tripping on m68k

2014-06-06 Thread Paul E. McKenney
On Fri, May 30, 2014 at 11:29:41AM +1000, Greg Ungerer wrote:
> On 29/05/14 23:11, One Thousand Gnomes wrote:
> > On Thu, 29 May 2014 12:08:32 +1000
> > Greg Ungerer  wrote:
> > 
> >> Hi All,
> >>
> >> Inside kernel/rcy/tree.c in __call_rcu() it does an alignment check on
> >> the head pointer passed in. This trips on m68k systems, because they only
> >> need alignment of 32bit quantities to 16bit boundaries.
> > 
> > __alignof perhaps ?
> 
> That might do. Change then becomes something like:
> 
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2467,7 +2467,7 @@ __call_rcu(struct rcu_head *head, void (*func)(struct 
> rcu_
> unsigned long flags;
> struct rcu_data *rdp;
> 
> -   WARN_ON_ONCE((unsigned long)head & 0x3); /* Misaligned rcu_head! */
> +   WARN_ON_ONCE((unsigned long)head & (__alignof__(head) - 1)); /* 
> Misaligned rcu_head! */

Hmmm...  The purpose of the check is to reserve the low-order bits to
allow RCU to classify callbacks as being time-critical or not.  RCU
can probably live with a single bit, but if there is some architecture
out there that simply refuses to do alignment, I need to know about it.

(See "git show 0bb7b59d6e2b8" for more info.)

So how about this instead?

 -   WARN_ON_ONCE((unsigned long)head & 0x1); /* Misaligned rcu_head! */

(Trying to remember if I have seen Linux kernel code that uses both
the lower bits...)

Thanx, Paul

> if (debug_rcu_head_queue(head)) {
> /* Probable double call_rcu(), so leak the callback. */
> ACCESS_ONCE(head->func) = rcu_leak_callback;
> 
> Thanks
> Greg
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: rcu alignment warning tripping on m68k

2014-05-29 Thread Greg Ungerer
On 29/05/14 23:11, One Thousand Gnomes wrote:
> On Thu, 29 May 2014 12:08:32 +1000
> Greg Ungerer  wrote:
> 
>> Hi All,
>>
>> Inside kernel/rcy/tree.c in __call_rcu() it does an alignment check on
>> the head pointer passed in. This trips on m68k systems, because they only
>> need alignment of 32bit quantities to 16bit boundaries.
> 
> __alignof perhaps ?

That might do. Change then becomes something like:

--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2467,7 +2467,7 @@ __call_rcu(struct rcu_head *head, void (*func)(struct rcu_
unsigned long flags;
struct rcu_data *rdp;
 
-   WARN_ON_ONCE((unsigned long)head & 0x3); /* Misaligned rcu_head! */
+   WARN_ON_ONCE((unsigned long)head & (__alignof__(head) - 1)); /* 
Misaligned rcu_head! */
if (debug_rcu_head_queue(head)) {
/* Probable double call_rcu(), so leak the callback. */
ACCESS_ONCE(head->func) = rcu_leak_callback;

Thanks
Greg


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: rcu alignment warning tripping on m68k

2014-05-29 Thread One Thousand Gnomes
On Thu, 29 May 2014 12:08:32 +1000
Greg Ungerer  wrote:

> Hi All,
> 
> Inside kernel/rcy/tree.c in __call_rcu() it does an alignment check on
> the head pointer passed in. This trips on m68k systems, because they only
> need alignment of 32bit quantities to 16bit boundaries.

__alignof perhaps ?

or just an ARCH_ define ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


rcu alignment warning tripping on m68k

2014-05-28 Thread Greg Ungerer
Hi All,

Inside kernel/rcy/tree.c in __call_rcu() it does an alignment check on
the head pointer passed in. This trips on m68k systems, because they only
need alignment of 32bit quantities to 16bit boundaries.

...
UDP hash table entries: 512 (order: 0, 8192 bytes)
UDP-Lite hash table entries: 512 (order: 0, 8192 bytes)
NET: Registered protocol family 1
ROMFS MTD (C) 2007 Red Hat, Inc.
[ cut here ]
WARNING: CPU: 0 PID: 18 at kernel/rcu/tree.c:2470 0x400577e0()
Modules linked in:
CPU: 0 PID: 18 Comm: kworker/u2:0 Not tainted 3.15.0-rc4-00013-g1c07b64-dirty 
#14
Stack from 418ada9c:
418ada9c 401e8336 400258e0 401c69c4 418ac000 418a86f6 4003bc0a 418a8390
418a86f6 4002590e 401e176b 09a6 400577e0 0009  400577e0
401e176b 09a6 401c69c4 418ac000 418a8390 418a8390 418ac000 40057928
418a86f6 40025bcc 40218fe0  401ddb56 401e17ad 02af 400261c2
418a86f6 40025bcc 0013 0030 0010 0013 418ac000 418adf88
418a8390 418adf70 400263f0 418a8000 418adf18 41885950  
Call Trace:
[<400258e0>] 0x400258e0
 [<401c69c4>] 0x401c69c4
 [<4003bc0a>] 0x4003bc0a
 [<4002590e>] 0x4002590e
 [<400577e0>] 0x400577e0


What is the best solution to fix?
Make the alignment check be only 16bits?

--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2467,7 +2467,7 @@ __call_rcu(struct rcu_head *head, void (*func)(struct rcu_
unsigned long flags;
struct rcu_data *rdp;
 
-   WARN_ON_ONCE((unsigned long)head & 0x3); /* Misaligned rcu_head! */
+   WARN_ON_ONCE((unsigned long)head & 0x1); /* Misaligned rcu_head! */
if (debug_rcu_head_queue(head)) {
/* Probable double call_rcu(), so leak the callback. */
ACCESS_ONCE(head->func) = rcu_leak_callback;


Or should we just get rid of this check altogether?

Regards
Greg

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/