[PATCH] slab: Fix off by one in object max number tests.

[PATCH] slab: Fix off by one in object max number tests.

[David Miller]

If freelist_idx_t is a byte, SLAB_OBJ_MAX_NUM should be 255 not 256,
and likewise if freelist_idx_t is a short, then it should be 65535 not
65536.

Fixes: a41adfa ("slab: introduce byte sized index for the freelist of a slab")
Signed-off-by: David S. Miller <davem@davemloft.net>
---

This was leading to all kinds of random crashes on sparc64 where PAGE_SIZE
is 8192.  One problem shown was that if spinlock debugging was enabled,
we'd get deadlocks in copy_pte_range() or do_wp_page() with the same cpu
already holding a lock it shouldn't hold, or the lock belonging to a
completely unrelated process.

diff --git a/mm/slab.c b/mm/slab.c
index 388cb1a..37de3a7 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -166,7 +166,7 @@ typedef unsigned char freelist_idx_t;
 typedef unsigned short freelist_idx_t;
 #endif
 
-#define SLAB_OBJ_MAX_NUM (1 << sizeof(freelist_idx_t) * BITS_PER_BYTE)
+#define SLAB_OBJ_MAX_NUM ((1 << sizeof(freelist_idx_t) * BITS_PER_BYTE) - 1)
 
 /*
  * true if a page was allocated from pfmemalloc reserves for network-based

Re: [PATCH] slab: Fix off by one in object max number tests.

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 05 May 2014 16:20:04 -0400 (EDT)

> 
> If freelist_idx_t is a byte, SLAB_OBJ_MAX_NUM should be 255 not 256,
> and likewise if freelist_idx_t is a short, then it should be 65535 not
> 65536.
> 
> Fixes: a41adfa ("slab: introduce byte sized index for the freelist of a slab")
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> 
> This was leading to all kinds of random crashes on sparc64 where PAGE_SIZE
> is 8192.  One problem shown was that if spinlock debugging was enabled,
> we'd get deadlocks in copy_pte_range() or do_wp_page() with the same cpu
> already holding a lock it shouldn't hold, or the lock belonging to a
> completely unrelated process.

It turns out that after some more testing, I'm still getting spinlock
debugging problems with this fix applied.

The change is still very much correct I think, however.

Re: [PATCH] slab: Fix off by one in object max number tests.

[Sam Ravnborg]

On Mon, May 05, 2014 at 04:57:56PM -0400, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Mon, 05 May 2014 16:20:04 -0400 (EDT)
> 
> > 
> > If freelist_idx_t is a byte, SLAB_OBJ_MAX_NUM should be 255 not 256,
> > and likewise if freelist_idx_t is a short, then it should be 65535 not
> > 65536.
> > 
> > Fixes: a41adfa ("slab: introduce byte sized index for the freelist of a slab")
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > ---
> > 
> > This was leading to all kinds of random crashes on sparc64 where PAGE_SIZE
> > is 8192.  One problem shown was that if spinlock debugging was enabled,
> > we'd get deadlocks in copy_pte_range() or do_wp_page() with the same cpu
> > already holding a lock it shouldn't hold, or the lock belonging to a
> > completely unrelated process.
> 
> It turns out that after some more testing, I'm still getting spinlock
> debugging problems with this fix applied.
> 
> The change is still very much correct I think, however.

There is a related patch in this area which I think is not yet applied.

See: https://lkml.org/lkml/2014/4/18/28

Maybe this is realted.

	Sam

Re: [PATCH] slab: Fix off by one in object max number tests.

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Mon, 5 May 2014 23:05:07 +0200

> On Mon, May 05, 2014 at 04:57:56PM -0400, David Miller wrote:
>> From: David Miller <davem@davemloft.net>
>> Date: Mon, 05 May 2014 16:20:04 -0400 (EDT)
>> 
>> > 
>> > If freelist_idx_t is a byte, SLAB_OBJ_MAX_NUM should be 255 not 256,
>> > and likewise if freelist_idx_t is a short, then it should be 65535 not
>> > 65536.
>> > 
>> > Fixes: a41adfa ("slab: introduce byte sized index for the freelist of a slab")
>> > Signed-off-by: David S. Miller <davem@davemloft.net>
>> > ---
>> > 
>> > This was leading to all kinds of random crashes on sparc64 where PAGE_SIZE
>> > is 8192.  One problem shown was that if spinlock debugging was enabled,
>> > we'd get deadlocks in copy_pte_range() or do_wp_page() with the same cpu
>> > already holding a lock it shouldn't hold, or the lock belonging to a
>> > completely unrelated process.
>> 
>> It turns out that after some more testing, I'm still getting spinlock
>> debugging problems with this fix applied.
>> 
>> The change is still very much correct I think, however.
> 
> There is a related patch in this area which I think is not yet applied.
> 
> See: https://lkml.org/lkml/2014/4/18/28
> 
> Maybe this is realted.

Thanks, I'm testing with that patch added as well.

Re: [PATCH] slab: Fix off by one in object max number tests.

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 05 May 2014 17:08:55 -0400 (EDT)

> From: Sam Ravnborg <sam@ravnborg.org>
> Date: Mon, 5 May 2014 23:05:07 +0200
> 
>> On Mon, May 05, 2014 at 04:57:56PM -0400, David Miller wrote:
>>> From: David Miller <davem@davemloft.net>
>>> Date: Mon, 05 May 2014 16:20:04 -0400 (EDT)
>>> 
>>> > 
>>> > If freelist_idx_t is a byte, SLAB_OBJ_MAX_NUM should be 255 not 256,
>>> > and likewise if freelist_idx_t is a short, then it should be 65535 not
>>> > 65536.
>>> > 
>>> > Fixes: a41adfa ("slab: introduce byte sized index for the freelist of a slab")
>>> > Signed-off-by: David S. Miller <davem@davemloft.net>
>>> > ---
>>> > 
>>> > This was leading to all kinds of random crashes on sparc64 where PAGE_SIZE
>>> > is 8192.  One problem shown was that if spinlock debugging was enabled,
>>> > we'd get deadlocks in copy_pte_range() or do_wp_page() with the same cpu
>>> > already holding a lock it shouldn't hold, or the lock belonging to a
>>> > completely unrelated process.
>>> 
>>> It turns out that after some more testing, I'm still getting spinlock
>>> debugging problems with this fix applied.
>>> 
>>> The change is still very much correct I think, however.
>> 
>> There is a related patch in this area which I think is not yet applied.
>> 
>> See: https://lkml.org/lkml/2014/4/18/28
>> 
>> Maybe this is realted.
> 
> Thanks, I'm testing with that patch added as well.

I can confirm that this fixes my problems completely.

Re: [PATCH] slab: Fix off by one in object max number tests.

[Linus Torvalds]

On Mon, May 5, 2014 at 8:25 PM, David Miller <davem@davemloft.net> wrote:
>
>> Sam Ravnborg <sam@ravnborg.org> wrote:
>>>
>>> There is a related patch in this area which I think is not yet applied.
>>>
>>> See: https://lkml.org/lkml/2014/4/18/28
>>>
>>> Maybe this is related.
>>
>> Thanks, I'm testing with that patch added as well.
>
> I can confirm that this fixes my problems completely.

Ugh. That patch has been out two weeks already. Pekka?

I guess I'll apply both patches directly.

              Linus

Re: [PATCH] slab: Fix off by one in object max number tests.

[Linus Torvalds]

On Mon, May 5, 2014 at 8:32 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I guess I'll apply both patches directly.

Applied and pushed out. Davem, I hope this means current -git works
for you on sparc64 again?

                Linus

Re: [PATCH] slab: Fix off by one in object max number tests.

[David Miller]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mon, 5 May 2014 20:46:32 -0700

> On Mon, May 5, 2014 at 8:32 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> I guess I'll apply both patches directly.
> 
> Applied and pushed out. Davem, I hope this means current -git works
> for you on sparc64 again?

Yes it absoultely should, and I'll double check to make sure.

Re: [PATCH] slab: Fix off by one in object max number tests.

[Pekka Enberg]

On Tue, May 6, 2014 at 6:32 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, May 5, 2014 at 8:25 PM, David Miller <davem@davemloft.net> wrote:
>>
>>> Sam Ravnborg <sam@ravnborg.org> wrote:
>>>>
>>>> There is a related patch in this area which I think is not yet applied.
>>>>
>>>> See: https://lkml.org/lkml/2014/4/18/28
>>>>
>>>> Maybe this is related.
>>>
>>> Thanks, I'm testing with that patch added as well.
>>
>> I can confirm that this fixes my problems completely.
>
> Ugh. That patch has been out two weeks already. Pekka?

Sorry, I missed that completely.

> I guess I'll apply both patches directly.

Thanks!