LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* "slab: Fix missing DEBUG_SLAB last user" breaks ARM
@ 2011-02-07 18:06 Russell King
  2011-02-08  5:44 ` Pekka Enberg
  0 siblings, 1 reply; 11+ messages in thread
From: Russell King @ 2011-02-07 18:06 UTC (permalink / raw)
  To: Shiyong Li, Pekka Enberg, Linux Kernel List

commit 5c5e3b33 appears to break ARM thusly:

| Mount-cache hash table entries: 512
| slab error in verify_redzone_free(): cache `idr_layer_cache': memory outside object was overwritten
| Backtrace:               
| [<c0227088>] (dump_backtrace+0x0/0x110) from [<c0431afc>] (dump_stack+0x18/0x1c)
| [<c0431ae4>] (dump_stack+0x0/0x1c) from [<c0293304>] (__slab_error+0x28/0x30)
| [<c02932dc>] (__slab_error+0x0/0x30) from [<c0293a74>] (cache_free_debugcheck+0x1c0/0x2b8)               
| [<c02938b4>] (cache_free_debugcheck+0x0/0x2b8) from [<c0293f78>] (kmem_cache_free+0x3c/0xc0)             
| [<c0293f3c>] (kmem_cache_free+0x0/0xc0) from [<c032b1c8>] (ida_get_new_above+0x19c/0x1c0)                
| [<c032b02c>] (ida_get_new_above+0x0/0x1c0) from [<c02af7ec>] (alloc_vfsmnt+0x54/0x144)                   
| [<c02af798>] (alloc_vfsmnt+0x0/0x144) from [<c0299830>] (vfs_kern_mount+0x30/0xec)                       
| [<c0299800>] (vfs_kern_mount+0x0/0xec) from [<c0299908>] (kern_mount_data+0x1c/0x20)                     
| [<c02998ec>] (kern_mount_data+0x0/0x20) from [<c02146c4>] (sysfs_init+0x68/0xc8)
| [<c021465c>] (sysfs_init+0x0/0xc8) from [<c02137d4>] (mnt_init+0x90/0x1b0)
| [<c0213744>] (mnt_init+0x0/0x1b0) from [<c0213388>] (vfs_caches_init+0x100/0x140)
| [<c0213288>] (vfs_caches_init+0x0/0x140) from [<c0208c0c>] (start_kernel+0x2e8/0x368)                    
| [<c0208924>] (start_kernel+0x0/0x368) from [<c0208034>] (__enable_mmu+0x0/0x2c)
| c0113268: redzone 1:0xd84156c5c032b3ac, redzone 2:0xd84156c5635688c0.
| slab error in cache_alloc_debugcheck_after(): cache `idr_layer_cache': double free, or memory outside object was overwritten
| ...
| c011307c: redzone 1:0x9f91102ffffffff, redzone 2:0x9f911029d74e35b
| slab: Internal list corruption detected in cache 'idr_layer_cache'(24), slabp c0113000(16). Hexdump:     
|                          
| 000: 20 4f 10 c0 20 4f 10 c0 7c 00 00 00 7c 30 11 c0
| 010: 10 00 00 00 10 00 00 00 00 00 c9 17 fe ff ff ff
| 020: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
| 030: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
| 040: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
| 050: fe ff ff ff fe ff ff ff fe ff ff ff 11 00 00 00
| 060: 12 00 00 00 13 00 00 00 14 00 00 00 15 00 00 00
| 070: 16 00 00 00 17 00 00 00 c0 88 56 63
| kernel BUG at /home/rmk/git/linux-2.6-rmk/mm/slab.c:2928!

This hasn't been noticed as I guess not many people use SLAB on ARM
anymore, and even less people probably have SLAB debugging enabled.
With SLAB debugging disabled, the system appears to behave correctly -
or maybe the problem is just hidden.

--
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-07 18:06 "slab: Fix missing DEBUG_SLAB last user" breaks ARM Russell King
@ 2011-02-08  5:44 ` Pekka Enberg
  2011-02-08 12:55   ` Russell King
  0 siblings, 1 reply; 11+ messages in thread
From: Pekka Enberg @ 2011-02-08  5:44 UTC (permalink / raw)
  To: Russell King
  Cc: Shiyong Li, Linux Kernel List, Christoph Lameter, David Rientjes

Hi!

On Mon, Feb 7, 2011 at 8:06 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> commit 5c5e3b33 appears to break ARM thusly:
>
> | Mount-cache hash table entries: 512
> | slab error in verify_redzone_free(): cache `idr_layer_cache': memory outside object was overwritten
> | Backtrace:
> | [<c0227088>] (dump_backtrace+0x0/0x110) from [<c0431afc>] (dump_stack+0x18/0x1c)
> | [<c0431ae4>] (dump_stack+0x0/0x1c) from [<c0293304>] (__slab_error+0x28/0x30)
> | [<c02932dc>] (__slab_error+0x0/0x30) from [<c0293a74>] (cache_free_debugcheck+0x1c0/0x2b8)
> | [<c02938b4>] (cache_free_debugcheck+0x0/0x2b8) from [<c0293f78>] (kmem_cache_free+0x3c/0xc0)
> | [<c0293f3c>] (kmem_cache_free+0x0/0xc0) from [<c032b1c8>] (ida_get_new_above+0x19c/0x1c0)
> | [<c032b02c>] (ida_get_new_above+0x0/0x1c0) from [<c02af7ec>] (alloc_vfsmnt+0x54/0x144)
> | [<c02af798>] (alloc_vfsmnt+0x0/0x144) from [<c0299830>] (vfs_kern_mount+0x30/0xec)
> | [<c0299800>] (vfs_kern_mount+0x0/0xec) from [<c0299908>] (kern_mount_data+0x1c/0x20)
> | [<c02998ec>] (kern_mount_data+0x0/0x20) from [<c02146c4>] (sysfs_init+0x68/0xc8)
> | [<c021465c>] (sysfs_init+0x0/0xc8) from [<c02137d4>] (mnt_init+0x90/0x1b0)
> | [<c0213744>] (mnt_init+0x0/0x1b0) from [<c0213388>] (vfs_caches_init+0x100/0x140)
> | [<c0213288>] (vfs_caches_init+0x0/0x140) from [<c0208c0c>] (start_kernel+0x2e8/0x368)
> | [<c0208924>] (start_kernel+0x0/0x368) from [<c0208034>] (__enable_mmu+0x0/0x2c)
> | c0113268: redzone 1:0xd84156c5c032b3ac, redzone 2:0xd84156c5635688c0.
> | slab error in cache_alloc_debugcheck_after(): cache `idr_layer_cache': double free, or memory outside object was overwritten
> | ...
> | c011307c: redzone 1:0x9f91102ffffffff, redzone 2:0x9f911029d74e35b
> | slab: Internal list corruption detected in cache 'idr_layer_cache'(24), slabp c0113000(16). Hexdump:
> |
> | 000: 20 4f 10 c0 20 4f 10 c0 7c 00 00 00 7c 30 11 c0
> | 010: 10 00 00 00 10 00 00 00 00 00 c9 17 fe ff ff ff
> | 020: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
> | 030: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
> | 040: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
> | 050: fe ff ff ff fe ff ff ff fe ff ff ff 11 00 00 00
> | 060: 12 00 00 00 13 00 00 00 14 00 00 00 15 00 00 00
> | 070: 16 00 00 00 17 00 00 00 c0 88 56 63
> | kernel BUG at /home/rmk/git/linux-2.6-rmk/mm/slab.c:2928!
>
> This hasn't been noticed as I guess not many people use SLAB on ARM
> anymore, and even less people probably have SLAB debugging enabled.
> With SLAB debugging disabled, the system appears to behave correctly -
> or maybe the problem is just hidden.

I thought the fix was for ARM, actually, but unfortunately the
changelog is somewhat terse (shame on me). Hmm?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-08  5:44 ` Pekka Enberg
@ 2011-02-08 12:55   ` Russell King
  2011-02-08 15:02     ` Christoph Lameter
  0 siblings, 1 reply; 11+ messages in thread
From: Russell King @ 2011-02-08 12:55 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Shiyong Li, Linux Kernel List, Christoph Lameter, David Rientjes

On Tue, Feb 08, 2011 at 07:44:56AM +0200, Pekka Enberg wrote:
> Hi!
> 
> On Mon, Feb 7, 2011 at 8:06 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> > commit 5c5e3b33 appears to break ARM thusly:
> >
> > | Mount-cache hash table entries: 512
> > | slab error in verify_redzone_free(): cache `idr_layer_cache': memory outside object was overwritten
> > | Backtrace:
> > | [<c0227088>] (dump_backtrace+0x0/0x110) from [<c0431afc>] (dump_stack+0x18/0x1c)
> > | [<c0431ae4>] (dump_stack+0x0/0x1c) from [<c0293304>] (__slab_error+0x28/0x30)
> > | [<c02932dc>] (__slab_error+0x0/0x30) from [<c0293a74>] (cache_free_debugcheck+0x1c0/0x2b8)
> > | [<c02938b4>] (cache_free_debugcheck+0x0/0x2b8) from [<c0293f78>] (kmem_cache_free+0x3c/0xc0)
> > | [<c0293f3c>] (kmem_cache_free+0x0/0xc0) from [<c032b1c8>] (ida_get_new_above+0x19c/0x1c0)
> > | [<c032b02c>] (ida_get_new_above+0x0/0x1c0) from [<c02af7ec>] (alloc_vfsmnt+0x54/0x144)
> > | [<c02af798>] (alloc_vfsmnt+0x0/0x144) from [<c0299830>] (vfs_kern_mount+0x30/0xec)
> > | [<c0299800>] (vfs_kern_mount+0x0/0xec) from [<c0299908>] (kern_mount_data+0x1c/0x20)
> > | [<c02998ec>] (kern_mount_data+0x0/0x20) from [<c02146c4>] (sysfs_init+0x68/0xc8)
> > | [<c021465c>] (sysfs_init+0x0/0xc8) from [<c02137d4>] (mnt_init+0x90/0x1b0)
> > | [<c0213744>] (mnt_init+0x0/0x1b0) from [<c0213388>] (vfs_caches_init+0x100/0x140)
> > | [<c0213288>] (vfs_caches_init+0x0/0x140) from [<c0208c0c>] (start_kernel+0x2e8/0x368)
> > | [<c0208924>] (start_kernel+0x0/0x368) from [<c0208034>] (__enable_mmu+0x0/0x2c)
> > | c0113268: redzone 1:0xd84156c5c032b3ac, redzone 2:0xd84156c5635688c0.
> > | slab error in cache_alloc_debugcheck_after(): cache `idr_layer_cache': double free, or memory outside object was overwritten
> > | ...
> > | c011307c: redzone 1:0x9f91102ffffffff, redzone 2:0x9f911029d74e35b
> > | slab: Internal list corruption detected in cache 'idr_layer_cache'(24), slabp c0113000(16). Hexdump:
> > |
> > | 000: 20 4f 10 c0 20 4f 10 c0 7c 00 00 00 7c 30 11 c0
> > | 010: 10 00 00 00 10 00 00 00 00 00 c9 17 fe ff ff ff
> > | 020: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
> > | 030: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
> > | 040: fe ff ff ff fe ff ff ff fe ff ff ff fe ff ff ff
> > | 050: fe ff ff ff fe ff ff ff fe ff ff ff 11 00 00 00
> > | 060: 12 00 00 00 13 00 00 00 14 00 00 00 15 00 00 00
> > | 070: 16 00 00 00 17 00 00 00 c0 88 56 63
> > | kernel BUG at /home/rmk/git/linux-2.6-rmk/mm/slab.c:2928!
> >
> > This hasn't been noticed as I guess not many people use SLAB on ARM
> > anymore, and even less people probably have SLAB debugging enabled.
> > With SLAB debugging disabled, the system appears to behave correctly -
> > or maybe the problem is just hidden.
> 
> I thought the fix was for ARM, actually, but unfortunately the
> changelog is somewhat terse (shame on me). Hmm?

As you say, the changelog is soo terse that we now have no idea why this
commit was created.

So, what's the fix here?  It looks to me like a revert is in order.

I suggest that if it does get reverted, a copy of my original bug report
is included in the revert commit so that all the details are available if
it needs revisiting.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-08 12:55   ` Russell King
@ 2011-02-08 15:02     ` Christoph Lameter
  2011-02-11  7:02       ` Pekka Enberg
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Lameter @ 2011-02-08 15:02 UTC (permalink / raw)
  To: Russell King; +Cc: Pekka Enberg, Shiyong Li, Linux Kernel List, David Rientjes

On Tue, 8 Feb 2011, Russell King wrote:

> > I thought the fix was for ARM, actually, but unfortunately the
> > changelog is somewhat terse (shame on me). Hmm?
>
> As you say, the changelog is soo terse that we now have no idea why this
> commit was created.
>
> So, what's the fix here?  It looks to me like a revert is in order.
>
> I suggest that if it does get reverted, a copy of my original bug report
> is included in the revert commit so that all the details are available if
> it needs revisiting.

Uhh.. That first hunk in commit 5c5e3b33b7cb959a401f823707bee006caadd76e
looks wrong to me. ralign is usually a power of two and greater than
alignof(unsigned long long). The & operation will result in 0 and never
exempt any caches.

diff --git a/mm/slab.c b/mm/slab.c
index bac0f4f..525c664 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2220,8 +2220,8 @@ kmem_cache_create (const char *name, size_t size,
size_t align,
        if (ralign < align) {
                ralign = align;
        }
-       /* disable debug if necessary */
-       if (ralign > __alignof__(unsigned long long))
+       /* disable debug if not aligning with REDZONE_ALIGN */
+       if (ralign & (__alignof__(unsigned long long) - 1))
                flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
        /*
         * 4) Store it.


Did you mean

	if (ralign > REDZONE_ALIGN) ...

?



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-08 15:02     ` Christoph Lameter
@ 2011-02-11  7:02       ` Pekka Enberg
  2011-02-11  9:14         ` Russell King
  0 siblings, 1 reply; 11+ messages in thread
From: Pekka Enberg @ 2011-02-11  7:02 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Russell King, Shiyong Li, Linux Kernel List, David Rientjes

On Tue, Feb 8, 2011 at 5:02 PM, Christoph Lameter <cl@linux.com> wrote:
> On Tue, 8 Feb 2011, Russell King wrote:
>
>> > I thought the fix was for ARM, actually, but unfortunately the
>> > changelog is somewhat terse (shame on me). Hmm?
>>
>> As you say, the changelog is soo terse that we now have no idea why this
>> commit was created.
>>
>> So, what's the fix here?  It looks to me like a revert is in order.
>>
>> I suggest that if it does get reverted, a copy of my original bug report
>> is included in the revert commit so that all the details are available if
>> it needs revisiting.
>
> Uhh.. That first hunk in commit 5c5e3b33b7cb959a401f823707bee006caadd76e
> looks wrong to me. ralign is usually a power of two and greater than
> alignof(unsigned long long). The & operation will result in 0 and never
> exempt any caches.
>
> diff --git a/mm/slab.c b/mm/slab.c
> index bac0f4f..525c664 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -2220,8 +2220,8 @@ kmem_cache_create (const char *name, size_t size,
> size_t align,
>        if (ralign < align) {
>                ralign = align;
>        }
> -       /* disable debug if necessary */
> -       if (ralign > __alignof__(unsigned long long))
> +       /* disable debug if not aligning with REDZONE_ALIGN */
> +       if (ralign & (__alignof__(unsigned long long) - 1))
>                flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
>        /*
>         * 4) Store it.
>
>
> Did you mean
>
>        if (ralign > REDZONE_ALIGN) ...
>
> ?

Ping. Any arm-capable tester out there that can verify this fix? OTOH,
since Shiyong Li hasn't showed up, I'm inclined to just revert the
commit...

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-11  7:02       ` Pekka Enberg
@ 2011-02-11  9:14         ` Russell King
  2011-02-11 15:20           ` Christoph Lameter
  0 siblings, 1 reply; 11+ messages in thread
From: Russell King @ 2011-02-11  9:14 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Christoph Lameter, Shiyong Li, Linux Kernel List, David Rientjes

On Fri, Feb 11, 2011 at 09:02:45AM +0200, Pekka Enberg wrote:
> On Tue, Feb 8, 2011 at 5:02 PM, Christoph Lameter <cl@linux.com> wrote:
> > Uhh.. That first hunk in commit 5c5e3b33b7cb959a401f823707bee006caadd76e
> > looks wrong to me. ralign is usually a power of two and greater than
> > alignof(unsigned long long). The & operation will result in 0 and never
> > exempt any caches.
> >
> > diff --git a/mm/slab.c b/mm/slab.c
> > index bac0f4f..525c664 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -2220,8 +2220,8 @@ kmem_cache_create (const char *name, size_t size,
> > size_t align,
> >        if (ralign < align) {
> >                ralign = align;
> >        }
> > -       /* disable debug if necessary */
> > -       if (ralign > __alignof__(unsigned long long))
> > +       /* disable debug if not aligning with REDZONE_ALIGN */
> > +       if (ralign & (__alignof__(unsigned long long) - 1))
> >                flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
> >        /*
> >         * 4) Store it.
> >
> >
> > Did you mean
> >
> >        if (ralign > REDZONE_ALIGN) ...
> >
> > ?
> 
> Ping. Any arm-capable tester out there that can verify this fix? OTOH,
> since Shiyong Li hasn't showed up, I'm inclined to just revert the
> commit...

What fix?  The above is a quote of the patch in 5c5e3b33.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-11  9:14         ` Russell King
@ 2011-02-11 15:20           ` Christoph Lameter
  2011-02-11 15:35             ` Russell King
  0 siblings, 1 reply; 11+ messages in thread
From: Christoph Lameter @ 2011-02-11 15:20 UTC (permalink / raw)
  To: Russell King; +Cc: Pekka Enberg, Shiyong Li, Linux Kernel List, David Rientjes

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1519 bytes --]

On Fri, 11 Feb 2011, Russell King wrote:

> On Fri, Feb 11, 2011 at 09:02:45AM +0200, Pekka Enberg wrote:
> > On Tue, Feb 8, 2011 at 5:02 PM, Christoph Lameter <cl@linux.com> wrote:
> > > Uhh.. That first hunk in commit 5c5e3b33b7cb959a401f823707bee006caadd76e
> > > looks wrong to me. ralign is usually a power of two and greater than
> > > alignof(unsigned long long). The & operation will result in 0 and never
> > > exempt any caches.
> > >
> > > diff --git a/mm/slab.c b/mm/slab.c
> > > index bac0f4f..525c664 100644
> > > --- a/mm/slab.c
> > > +++ b/mm/slab.c
> > > @@ -2220,8 +2220,8 @@ kmem_cache_create (const char *name, size_t size,
> > > size_t align,
> > >        if (ralign < align) {
> > >                ralign = align;
> > >        }
> > > -       /* disable debug if necessary */
> > > -       if (ralign > __alignof__(unsigned long long))
> > > +       /* disable debug if not aligning with REDZONE_ALIGN */
> > > +       if (ralign & (__alignof__(unsigned long long) - 1))
> > >                flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
> > >        /*
> > >         * 4) Store it.
> > >
> > >
> > > Did you mean
> > >
> > >        if (ralign > REDZONE_ALIGN) ...
> > >
> > > ?
> >
> > Ping. Any arm-capable tester out there that can verify this fix? OTOH,
> > since Shiyong Li hasn't showed up, I'm inclined to just revert the
> > commit...
>
> What fix?  The above is a quote of the patch in 5c5e3b33.

He was referring to the change of the if statement.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-11 15:20           ` Christoph Lameter
@ 2011-02-11 15:35             ` Russell King
       [not found]               ` <AANLkTimDUf52xdHn22=f1J=Xn88sqPEg9P6+ceas1h1k@mail.gmail.com>
  0 siblings, 1 reply; 11+ messages in thread
From: Russell King @ 2011-02-11 15:35 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, Shiyong Li, Linux Kernel List, David Rientjes

On Fri, Feb 11, 2011 at 09:20:27AM -0600, Christoph Lameter wrote:
> On Fri, 11 Feb 2011, Russell King wrote:
> 
> > On Fri, Feb 11, 2011 at 09:02:45AM +0200, Pekka Enberg wrote:
> > > On Tue, Feb 8, 2011 at 5:02 PM, Christoph Lameter <cl@linux.com> wrote:
> > > > Uhh.. That first hunk in commit 5c5e3b33b7cb959a401f823707bee006caadd76e
> > > > looks wrong to me. ralign is usually a power of two and greater than
> > > > alignof(unsigned long long). The & operation will result in 0 and never
> > > > exempt any caches.
> > > >
> > > > diff --git a/mm/slab.c b/mm/slab.c
> > > > index bac0f4f..525c664 100644
> > > > --- a/mm/slab.c
> > > > +++ b/mm/slab.c
> > > > @@ -2220,8 +2220,8 @@ kmem_cache_create (const char *name, size_t size,
> > > > size_t align,
> > > >        if (ralign < align) {
> > > >                ralign = align;
> > > >        }
> > > > -       /* disable debug if necessary */
> > > > -       if (ralign > __alignof__(unsigned long long))
> > > > +       /* disable debug if not aligning with REDZONE_ALIGN */
> > > > +       if (ralign & (__alignof__(unsigned long long) - 1))
> > > >                flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
> > > >        /*
> > > >         * 4) Store it.
> > > >
> > > >
> > > > Did you mean
> > > >
> > > >        if (ralign > REDZONE_ALIGN) ...
> > > >
> > > > ?
> > >
> > > Ping. Any arm-capable tester out there that can verify this fix? OTOH,
> > > since Shiyong Li hasn't showed up, I'm inclined to just revert the
> > > commit...
> >
> > What fix?  The above is a quote of the patch in 5c5e3b33.
> 
> He was referring to the change of the if statement.

Replacing what with what?

	if (ralign & (__alignof__(unsigned long long) - 1))
with
	if (ralign > __alignof__(unsigned long long))
or
	if (ralign > REDZONE_ALIGN)

?  A patch is a wonderful way to describe the change to be tested.


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
       [not found]               ` <AANLkTimDUf52xdHn22=f1J=Xn88sqPEg9P6+ceas1h1k@mail.gmail.com>
@ 2011-02-14 15:40                 ` Russell King
  2011-02-14 15:43                   ` Pekka Enberg
  0 siblings, 1 reply; 11+ messages in thread
From: Russell King @ 2011-02-14 15:40 UTC (permalink / raw)
  To: ShiYong LI
  Cc: Christoph Lameter, Pekka Enberg, Shiyong Li, Linux Kernel List,
	David Rientjes

On Mon, Feb 14, 2011 at 11:10:45AM +0800, ShiYong LI wrote:
> Hi all,
> 
> Sorry for my late reply.
> 
> My commit is created to enable ARM with 128 byte cache line support last
> information dump while slab corruption occurs. I've verified it works fine
> on ARMv7.

Mainline only supports 32 byte and 64 byte cache lines on ARM.  It does
not support 128 byte cache lines, so maybe your attempts are the wrong
approach.  Can you please post your 128 byte cache line support patches
to the ARM kernel mailing list for review.

In the mean time, because your patch causes an oopsing regression, I
strongly suggest that it is reverted until we start supporting 128 byte
cache lines on ARM in mainline.

> Russell,
> 
> Can you provide more information for the problem? What kind of ARM are you
> using ?

ARM926, which has a cache line size of 32.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-14 15:40                 ` Russell King
@ 2011-02-14 15:43                   ` Pekka Enberg
  2011-02-14 18:07                     ` Pekka Enberg
  0 siblings, 1 reply; 11+ messages in thread
From: Pekka Enberg @ 2011-02-14 15:43 UTC (permalink / raw)
  To: Russell King
  Cc: ShiYong LI, Christoph Lameter, Shiyong Li, Linux Kernel List,
	David Rientjes

On Mon, Feb 14, 2011 at 5:40 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> Mainline only supports 32 byte and 64 byte cache lines on ARM.  It does
> not support 128 byte cache lines, so maybe your attempts are the wrong
> approach.  Can you please post your 128 byte cache line support patches
> to the ARM kernel mailing list for review.
>
> In the mean time, because your patch causes an oopsing regression, I
> strongly suggest that it is reverted until we start supporting 128 byte
> cache lines on ARM in mainline.

Agreed. I'll queue a revert. Thanks, Russell!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: "slab: Fix missing DEBUG_SLAB last user" breaks ARM
  2011-02-14 15:43                   ` Pekka Enberg
@ 2011-02-14 18:07                     ` Pekka Enberg
  0 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2011-02-14 18:07 UTC (permalink / raw)
  To: Russell King
  Cc: ShiYong LI, Christoph Lameter, Shiyong Li, Linux Kernel List,
	David Rientjes

On Mon, Feb 14, 2011 at 5:43 PM, Pekka Enberg <penberg@kernel.org> wrote:
> On Mon, Feb 14, 2011 at 5:40 PM, Russell King <rmk@arm.linux.org.uk> wrote:
>> Mainline only supports 32 byte and 64 byte cache lines on ARM.  It does
>> not support 128 byte cache lines, so maybe your attempts are the wrong
>> approach.  Can you please post your 128 byte cache line support patches
>> to the ARM kernel mailing list for review.
>>
>> In the mean time, because your patch causes an oopsing regression, I
>> strongly suggest that it is reverted until we start supporting 128 byte
>> cache lines on ARM in mainline.
>
> Agreed. I'll queue a revert. Thanks, Russell!

It's this commit queued for linux-next:

http://git.kernel.org/?p=linux/kernel/git/penberg/slab-2.6.git;a=commit;h=3ff84a7f36554b257cd57325b1a7c1fa4b49fbe3

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2011-02-14 18:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-07 18:06 "slab: Fix missing DEBUG_SLAB last user" breaks ARM Russell King
2011-02-08  5:44 ` Pekka Enberg
2011-02-08 12:55   ` Russell King
2011-02-08 15:02     ` Christoph Lameter
2011-02-11  7:02       ` Pekka Enberg
2011-02-11  9:14         ` Russell King
2011-02-11 15:20           ` Christoph Lameter
2011-02-11 15:35             ` Russell King
     [not found]               ` <AANLkTimDUf52xdHn22=f1J=Xn88sqPEg9P6+ceas1h1k@mail.gmail.com>
2011-02-14 15:40                 ` Russell King
2011-02-14 15:43                   ` Pekka Enberg
2011-02-14 18:07                     ` Pekka Enberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).