Hi all,
While fuzzing with trinity inside a KVM tools guest running the latest -next
kernel I've stumbled on the following spew:
[ 791.659908] BUG: unable to handle kernel paging request at ffff880302e12000
[ 791.661580] IP: memset (arch/x86/lib/memset_64.S:83)
[ 791.661580] PGD 17b7d067 PUD 704947067 PMD 70492f067 PTE 8000000302e12060
[ 791.661580] Oops: 0002 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 791.661580] Dumping ftrace buffer:
[ 791.661580] (ftrace buffer empty)
[ 791.667964] Modules linked in:
[ 791.667964] CPU: 13 PID: 10630 Comm: trinity-c20 Not tainted 3.16.0-rc2-next-20140624-sasha-00024-g332b58d #726
[ 791.669480] task: ffff8803d5123000 ti: ffff8803ba460000 task.ti: ffff8803ba460000
[ 791.669480] RIP: memset (arch/x86/lib/memset_64.S:83)
[ 791.669480] RSP: 0018:ffff8803ba463b18 EFLAGS: 00010003
[ 791.669480] RAX: 6b6b6b6b6b6b6b6b RBX: ffff880036851540 RCX: 0000000000000068
[ 791.669480] RDX: 0000000000002a77 RSI: 000000000000006b RDI: ffff880302e12000
[ 791.669480] RBP: ffff8803ba463b40 R08: 0000000000000001 R09: 0000000000000000
[ 791.669480] R10: ffff880302e11000 R11: ffffffffffffffd8 R12: ffff880302e11000
[ 791.669480] R13: 00000000000000bb R14: ffff880302e11000 R15: ffffffffffffffff
[ 791.669480] FS: 00007f37693b3700(0000) GS:ffff880334e00000(0000) knlGS:0000000000000000
[ 791.669480] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 791.669480] CR2: ffff880302e12000 CR3: 00000003b744f000 CR4: 00000000000006a0
[ 791.669480] Stack:
[ 791.669480] ffffffff902f4273 ffff8803ba463b30 ffff880036851540 ffff880302e11000
[ 791.669480] ffffea000c0b8440 ffff8803ba463b60 ffffffff902f48b0 ffff880036851540
[ 791.669480] ffff880302e11000 ffff8803ba463bc0 ffffffff902f6886 00000000000000d0
[ 791.669480] Call Trace:
[ 791.669480] ? init_object (mm/slub.c:665)
[ 791.669480] setup_object.isra.34 (mm/slub.c:1008 mm/slub.c:1373)
[ 791.669480] new_slab (mm/slub.c:278 mm/slub.c:1412)
[ 791.669480] __slab_alloc (mm/slub.c:2186 mm/slub.c:2344)
[ 791.690803] ? kvm_clock_read (./arch/x86/include/asm/preempt.h:90 arch/x86/kernel/kvmclock.c:86)
[ 791.690803] ? copy_process (kernel/fork.c:306 kernel/fork.c:1193)
[ 791.690803] ? sched_clock (./arch/x86/include/asm/paravirt.h:192 arch/x86/kernel/tsc.c:305)
[ 791.690803] ? get_parent_ip (kernel/sched/core.c:2550)
[ 791.690803] kmem_cache_alloc_node (mm/slub.c:2417 mm/slub.c:2486)
[ 791.690803] ? sched_clock_cpu (kernel/sched/clock.c:311)
[ 791.690803] ? copy_process (kernel/fork.c:306 kernel/fork.c:1193)
[ 791.690803] copy_process (kernel/fork.c:306 kernel/fork.c:1193)
[ 791.690803] ? kvm_clock_read (./arch/x86/include/asm/preempt.h:90 arch/x86/kernel/kvmclock.c:86)
[ 791.690803] ? sched_clock (./arch/x86/include/asm/paravirt.h:192 arch/x86/kernel/tsc.c:305)
[ 791.690803] ? sched_clock_local (kernel/sched/clock.c:214)
[ 791.690803] ? put_lock_stats.isra.12 (./arch/x86/include/asm/preempt.h:98 kernel/locking/lockdep.c:254)
[ 791.690803] do_fork (kernel/fork.c:1609)
[ 791.690803] ? get_parent_ip (kernel/sched/core.c:2550)
[ 791.690803] ? context_tracking_user_exit (./arch/x86/include/asm/paravirt.h:809 (discriminator 2) kernel/context_tracking.c:184 (discriminator 2))
[ 791.690803] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[ 791.690803] ? trace_hardirqs_on_caller (kernel/locking/lockdep.c:2557 kernel/locking/lockdep.c:2599)
[ 791.690803] ? trace_hardirqs_on (kernel/locking/lockdep.c:2607)
[ 791.690803] SyS_clone (kernel/fork.c:1695)
[ 791.690803] stub_clone (arch/x86/kernel/entry_64.S:637)
[ 791.690803] ? tracesys (arch/x86/kernel/entry_64.S:542)
[ 791.690803] Code: b8 01 01 01 01 01 01 01 01 48 0f af c1 41 89 f9 41 83 e1 07 75 70 48 89 d1 48 c1 e9 06 74 39 66 0f 1f 84 00 00 00 00 00 48 ff c9 <48> 89 07 48 89 47 08 48 89 47 10 48 89 47 18 48 89 47 20 48 89
All code
========
0: b8 01 01 01 01 mov $0x1010101,%eax
5: 01 01 add %eax,(%rcx)
7: 01 01 add %eax,(%rcx)
9: 48 0f af c1 imul %rcx,%rax
d: 41 89 f9 mov %edi,%r9d
10: 41 83 e1 07 and $0x7,%r9d
14: 75 70 jne 0x86
16: 48 89 d1 mov %rdx,%rcx
19: 48 c1 e9 06 shr $0x6,%rcx
1d: 74 39 je 0x58
1f: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1)
26: 00 00
28: 48 ff c9 dec %rcx
2b:* 48 89 07 mov %rax,(%rdi) <-- trapping instruction
2e: 48 89 47 08 mov %rax,0x8(%rdi)
32: 48 89 47 10 mov %rax,0x10(%rdi)
36: 48 89 47 18 mov %rax,0x18(%rdi)
3a: 48 89 47 20 mov %rax,0x20(%rdi)
3e: 48 89 00 mov %rax,(%rax)
Code starting with the faulting instruction
===========================================
0: 48 89 07 mov %rax,(%rdi)
3: 48 89 47 08 mov %rax,0x8(%rdi)
7: 48 89 47 10 mov %rax,0x10(%rdi)
b: 48 89 47 18 mov %rax,0x18(%rdi)
f: 48 89 47 20 mov %rax,0x20(%rdi)
13: 48 89 00 mov %rax,(%rax)
[ 791.690803] RIP memset (arch/x86/lib/memset_64.S:83)
[ 791.690803] RSP <ffff8803ba463b18>
[ 791.690803] CR2: ffff880302e12000
Thanks,
Sasha
On Wed, 25 Jun 2014, Sasha Levin wrote:
> [ 791.669480] ? init_object (mm/slub.c:665)
> [ 791.669480] setup_object.isra.34 (mm/slub.c:1008 mm/slub.c:1373)
> [ 791.669480] new_slab (mm/slub.c:278 mm/slub.c:1412)
So we just got a new page from the page allocator but somehow cannot
write to it. This is the first write access to the page.
On Wed, 25 Jun 2014, Christoph Lameter wrote:
> On Wed, 25 Jun 2014, Sasha Levin wrote:
>
> > [ 791.669480] ? init_object (mm/slub.c:665)
> > [ 791.669480] setup_object.isra.34 (mm/slub.c:1008 mm/slub.c:1373)
> > [ 791.669480] new_slab (mm/slub.c:278 mm/slub.c:1412)
>
> So we just got a new page from the page allocator but somehow cannot
> write to it. This is the first write access to the page.
>
I'd be inclined to think that this was a result of "slub: reduce duplicate
creation on the first object" from -mm[*] that was added the day before
Sasha reported the problem.
It's not at all clear to me that that patch is correct. Wei?
Sasha, with a revert of that patch, does this reproduce?
[*] http://ozlabs.org/~akpm/mmotm/broken-out/slub-reduce-duplicate-creation-on-the-first-object.patch
On Mon, Jun 30, 2014 at 03:03:21PM -0700, David Rientjes wrote:
>On Wed, 25 Jun 2014, Christoph Lameter wrote:
>
>> On Wed, 25 Jun 2014, Sasha Levin wrote:
>>
>> > [ 791.669480] ? init_object (mm/slub.c:665)
>> > [ 791.669480] setup_object.isra.34 (mm/slub.c:1008 mm/slub.c:1373)
>> > [ 791.669480] new_slab (mm/slub.c:278 mm/slub.c:1412)
>>
>> So we just got a new page from the page allocator but somehow cannot
>> write to it. This is the first write access to the page.
>>
>
>I'd be inclined to think that this was a result of "slub: reduce duplicate
>creation on the first object" from -mm[*] that was added the day before
>Sasha reported the problem.
>
>It's not at all clear to me that that patch is correct. Wei?
>
>Sasha, with a revert of that patch, does this reproduce?
>
> [*] http://ozlabs.org/~akpm/mmotm/broken-out/slub-reduce-duplicate-creation-on-the-first-object.patch
David,
So sad to see the error after applying my patch. In which case this is
triggered? The kernel with this patch runs fine on my laptop. Maybe there is
some corner case I missed? If you could tell me the way you reproduce it, I
would have a try on my side.
I did a simple test for this patch, my test code and result is attached.
1. kmem_cache.c
The test module.
2. kmem_log.txt
In this log, you can see 26 objects are initialized once exactly, while
without this patch, the first object will be initialized twice.
Fetch a cache from kmem_cache
new_slab: page->objects is 26
new_slab: setup on ffff880097038000, ffff8800970384e0
init_once: [00]ffff880097038000 is created
new_slab: setup on ffff8800970384e0, ffff8800970389c0
init_once: [01]ffff8800970384e0 is created
new_slab: setup on ffff8800970389c0, ffff880097038ea0
init_once: [02]ffff8800970389c0 is created
new_slab: setup on ffff880097038ea0, ffff880097039380
init_once: [03]ffff880097038ea0 is created
new_slab: setup on ffff880097039380, ffff880097039860
init_once: [04]ffff880097039380 is created
new_slab: setup on ffff880097039860, ffff880097039d40
init_once: [05]ffff880097039860 is created
new_slab: setup on ffff880097039d40, ffff88009703a220
init_once: [06]ffff880097039d40 is created
new_slab: setup on ffff88009703a220, ffff88009703a700
init_once: [07]ffff88009703a220 is created
new_slab: setup on ffff88009703a700, ffff88009703abe0
init_once: [08]ffff88009703a700 is created
new_slab: setup on ffff88009703abe0, ffff88009703b0c0
init_once: [09]ffff88009703abe0 is created
new_slab: setup on ffff88009703b0c0, ffff88009703b5a0
init_once: [10]ffff88009703b0c0 is created
new_slab: setup on ffff88009703b5a0, ffff88009703ba80
init_once: [11]ffff88009703b5a0 is created
new_slab: setup on ffff88009703ba80, ffff88009703bf60
init_once: [12]ffff88009703ba80 is created
new_slab: setup on ffff88009703bf60, ffff88009703c440
init_once: [13]ffff88009703bf60 is created
new_slab: setup on ffff88009703c440, ffff88009703c920
init_once: [14]ffff88009703c440 is created
new_slab: setup on ffff88009703c920, ffff88009703ce00
init_once: [15]ffff88009703c920 is created
new_slab: setup on ffff88009703ce00, ffff88009703d2e0
init_once: [16]ffff88009703ce00 is created
new_slab: setup on ffff88009703d2e0, ffff88009703d7c0
init_once: [17]ffff88009703d2e0 is created
new_slab: setup on ffff88009703d7c0, ffff88009703dca0
init_once: [18]ffff88009703d7c0 is created
new_slab: setup on ffff88009703dca0, ffff88009703e180
init_once: [19]ffff88009703dca0 is created
new_slab: setup on ffff88009703e180, ffff88009703e660
init_once: [20]ffff88009703e180 is created
new_slab: setup on ffff88009703e660, ffff88009703eb40
init_once: [21]ffff88009703e660 is created
new_slab: setup on ffff88009703eb40, ffff88009703f020
init_once: [22]ffff88009703eb40 is created
new_slab: setup on ffff88009703f020, ffff88009703f500
init_once: [23]ffff88009703f020 is created
new_slab: setup on ffff88009703f500, ffff88009703f9e0
init_once: [24]ffff88009703f500 is created
new_slab: do it again? ffff88009703f9e0
init_once: [25]ffff88009703f9e0 is created
--
Richard Yang
Help you, Help me
On Mon, 30 Jun 2014, David Rientjes wrote:
> It's not at all clear to me that that patch is correct. Wei?
Looks ok to me. But I do not like the convoluted code in new_slab() which
Wei's patch does not make easier to read. Makes it difficult for the
reader to see whats going on.
Lets drop the use of the variable named "last".
Subject: slub: Only call setup_object once for each object
Modify the logic for object initialization to be less convoluted
and initialize an object only once.
Signed-off-by: Christoph Lameter <[email protected]>
Index: linux/mm/slub.c
===================================================================
--- linux.orig/mm/slub.c 2014-07-01 09:50:02.486846653 -0500
+++ linux/mm/slub.c 2014-07-01 09:52:07.918802585 -0500
@@ -1409,7 +1409,6 @@ static struct page *new_slab(struct kmem
{
struct page *page;
void *start;
- void *last;
void *p;
int order;
@@ -1432,15 +1431,11 @@ static struct page *new_slab(struct kmem
if (unlikely(s->flags & SLAB_POISON))
memset(start, POISON_INUSE, PAGE_SIZE << order);
- last = start;
for_each_object(p, s, start, page->objects) {
- setup_object(s, page, last);
- set_freepointer(s, last, p);
- last = p;
+ setup_object(s, page, p);
+ set_freepointer(s, p, p + s->size);
}
- setup_object(s, page, last);
- set_freepointer(s, last, NULL);
-
+ set_freepointer(s, start + (page->objects - 1) * s->size, NULL);
page->freelist = start;
page->inuse = page->objects;
page->frozen = 1;
On Tue, 1 Jul 2014 09:58:52 -0500 (CDT) Christoph Lameter <[email protected]> wrote:
> On Mon, 30 Jun 2014, David Rientjes wrote:
>
> > It's not at all clear to me that that patch is correct. Wei?
>
> Looks ok to me. But I do not like the convoluted code in new_slab() which
> Wei's patch does not make easier to read. Makes it difficult for the
> reader to see whats going on.
>
> Lets drop the use of the variable named "last".
>
>
> Subject: slub: Only call setup_object once for each object
>
> Modify the logic for object initialization to be less convoluted
> and initialize an object only once.
>
Well, um. Wei's changelog was much better:
: When a kmem_cache is created with ctor, each object in the kmem_cache will
: be initialized before use. In the slub implementation, the first object
: will be initialized twice.
:
: This patch avoids the duplication of initialization of the first object.
:
: Fixes commit 7656c72b5a63: ("SLUB: add macros for scanning objects in a
: slab").
I can copy that text over and add the reported-by etc (ho hum) but I
have a tiny feeling that this patch hasn't been rigorously tested?
Perhaps someone (Wei?) can do that?
And we still don't know why Sasha's kernel went oops.
On 07/01/2014 05:49 PM, Andrew Morton wrote:
> On Tue, 1 Jul 2014 09:58:52 -0500 (CDT) Christoph Lameter <[email protected]> wrote:
>
>> On Mon, 30 Jun 2014, David Rientjes wrote:
>>
>>> It's not at all clear to me that that patch is correct. Wei?
>>
>> Looks ok to me. But I do not like the convoluted code in new_slab() which
>> Wei's patch does not make easier to read. Makes it difficult for the
>> reader to see whats going on.
>>
>> Lets drop the use of the variable named "last".
>>
>>
>> Subject: slub: Only call setup_object once for each object
>>
>> Modify the logic for object initialization to be less convoluted
>> and initialize an object only once.
>>
>
> Well, um. Wei's changelog was much better:
>
> : When a kmem_cache is created with ctor, each object in the kmem_cache will
> : be initialized before use. In the slub implementation, the first object
> : will be initialized twice.
> :
> : This patch avoids the duplication of initialization of the first object.
> :
> : Fixes commit 7656c72b5a63: ("SLUB: add macros for scanning objects in a
> : slab").
>
> I can copy that text over and add the reported-by etc (ho hum) but I
> have a tiny feeling that this patch hasn't been rigorously tested?
> Perhaps someone (Wei?) can do that?
>
> And we still don't know why Sasha's kernel went oops.
I only saw this oops once, and after David's message yesterday I tried reverting
the patch he pointed out, but not much changed.
Is there a better way to stress test slub?
Thanks,
Sasha
On Tue, Jul 01, 2014 at 09:58:52AM -0500, Christoph Lameter wrote:
>On Mon, 30 Jun 2014, David Rientjes wrote:
>
>> It's not at all clear to me that that patch is correct. Wei?
>
>Looks ok to me. But I do not like the convoluted code in new_slab() which
>Wei's patch does not make easier to read. Makes it difficult for the
>reader to see whats going on.
My patch is somewhat convoluted since I wanted to preserve the original logic
and make minimal change. And yes, it looks not that nice to audience.
I feel a little hurt by this patch. What I found and worked is gone with this
patch.
>
>Lets drop the use of the variable named "last".
>
>
>Subject: slub: Only call setup_object once for each object
>
>Modify the logic for object initialization to be less convoluted
>and initialize an object only once.
>
>Signed-off-by: Christoph Lameter <[email protected]>
>
>Index: linux/mm/slub.c
>===================================================================
>--- linux.orig/mm/slub.c 2014-07-01 09:50:02.486846653 -0500
>+++ linux/mm/slub.c 2014-07-01 09:52:07.918802585 -0500
>@@ -1409,7 +1409,6 @@ static struct page *new_slab(struct kmem
> {
> struct page *page;
> void *start;
>- void *last;
> void *p;
> int order;
>
>@@ -1432,15 +1431,11 @@ static struct page *new_slab(struct kmem
> if (unlikely(s->flags & SLAB_POISON))
> memset(start, POISON_INUSE, PAGE_SIZE << order);
>
>- last = start;
> for_each_object(p, s, start, page->objects) {
>- setup_object(s, page, last);
>- set_freepointer(s, last, p);
>- last = p;
>+ setup_object(s, page, p);
>+ set_freepointer(s, p, p + s->size);
> }
>- setup_object(s, page, last);
>- set_freepointer(s, last, NULL);
>-
>+ set_freepointer(s, start + (page->objects - 1) * s->size, NULL);
> page->freelist = start;
> page->inuse = page->objects;
> page->frozen = 1;
--
Richard Yang
Help you, Help me
On Tue, Jul 01, 2014 at 02:49:47PM -0700, Andrew Morton wrote:
>On Tue, 1 Jul 2014 09:58:52 -0500 (CDT) Christoph Lameter <[email protected]> wrote:
>
>> On Mon, 30 Jun 2014, David Rientjes wrote:
>>
>> > It's not at all clear to me that that patch is correct. Wei?
>>
>> Looks ok to me. But I do not like the convoluted code in new_slab() which
>> Wei's patch does not make easier to read. Makes it difficult for the
>> reader to see whats going on.
>>
>> Lets drop the use of the variable named "last".
>>
>>
>> Subject: slub: Only call setup_object once for each object
>>
>> Modify the logic for object initialization to be less convoluted
>> and initialize an object only once.
>>
>
>Well, um. Wei's changelog was much better:
>
>: When a kmem_cache is created with ctor, each object in the kmem_cache will
>: be initialized before use. In the slub implementation, the first object
>: will be initialized twice.
>:
>: This patch avoids the duplication of initialization of the first object.
>:
>: Fixes commit 7656c72b5a63: ("SLUB: add macros for scanning objects in a
>: slab").
>
>I can copy that text over and add the reported-by etc (ho hum) but I
>have a tiny feeling that this patch hasn't been rigorously tested?
>Perhaps someone (Wei?) can do that?
Ok, I will apply this one and give a shot.
>
>And we still don't know why Sasha's kernel went oops.
Yep, if there is some procedure to reproduce it, I'd like to do it at my side.
--
Richard Yang
Help you, Help me
On Wed, 2 Jul 2014, Wei Yang wrote:
> My patch is somewhat convoluted since I wanted to preserve the original logic
> and make minimal change. And yes, it looks not that nice to audience.
Well I was the author of the initial "convoluted" logic.
> I feel a little hurt by this patch. What I found and worked is gone with this
> patch.
Ok how about giving this one additional revision. Maybe you can make the
function even easier to read? F.e. the setting of the NULL pointer at the
end of the loop is ugly.
On Tue, 1 Jul 2014, Sasha Levin wrote:
> Is there a better way to stress test slub?
The typical way to test is by stressing the network subsystem
with small packets that require small allocations. Or do a filesystem
test that requires lots of metadata (file creations, removal, renames
etc).
But I also posted some in kernel benchmarks a while back
https://lkml.org/lkml/2009/10/13/459
Pekka had a project going to get these merged.
https://lkml.org/lkml/2009/11/29/17
On Tue, 1 Jul 2014, Andrew Morton wrote:
> I can copy that text over and add the reported-by etc (ho hum) but I
> have a tiny feeling that this patch hasn't been rigorously tested?
The testing so far was to verify that a kernel successfully builds with
the patch and then booted upo in a kvm instance.
On Tue, Jul 01, 2014 at 02:49:47PM -0700, Andrew Morton wrote:
>On Tue, 1 Jul 2014 09:58:52 -0500 (CDT) Christoph Lameter <[email protected]> wrote:
>
>> On Mon, 30 Jun 2014, David Rientjes wrote:
>>
>> > It's not at all clear to me that that patch is correct. Wei?
>>
>> Looks ok to me. But I do not like the convoluted code in new_slab() which
>> Wei's patch does not make easier to read. Makes it difficult for the
>> reader to see whats going on.
>>
>> Lets drop the use of the variable named "last".
>>
>>
>> Subject: slub: Only call setup_object once for each object
>>
>> Modify the logic for object initialization to be less convoluted
>> and initialize an object only once.
>>
>
>Well, um. Wei's changelog was much better:
>
>: When a kmem_cache is created with ctor, each object in the kmem_cache will
>: be initialized before use. In the slub implementation, the first object
>: will be initialized twice.
>:
>: This patch avoids the duplication of initialization of the first object.
>:
>: Fixes commit 7656c72b5a63: ("SLUB: add macros for scanning objects in a
>: slab").
>
>I can copy that text over and add the reported-by etc (ho hum) but I
>have a tiny feeling that this patch hasn't been rigorously tested?
>Perhaps someone (Wei?) can do that?
The result of the simple test is the same. And my laptop works a whole day
with this patch.
Thanks
>
>And we still don't know why Sasha's kernel went oops.
--
Richard Yang
Help you, Help me
On Wed, Jul 02, 2014 at 09:20:20AM -0500, Christoph Lameter wrote:
>On Wed, 2 Jul 2014, Wei Yang wrote:
>
>> My patch is somewhat convoluted since I wanted to preserve the original logic
>> and make minimal change. And yes, it looks not that nice to audience.
>
>Well I was the author of the initial "convoluted" logic.
>
>> I feel a little hurt by this patch. What I found and worked is gone with this
>> patch.
>
>Ok how about giving this one additional revision. Maybe you can make the
>function even easier to read? F.e. the setting of the NULL pointer at the
>end of the loop is ugly.
Hi, Christoph
Here is my refined version, hope this is more friendly to the audience.
>From 3f4fdeab600e53fdcbd65c817db3aa560ac16bfb Mon Sep 17 00:00:00 2001
From: Wei Yang <[email protected]>
Date: Tue, 24 Jun 2014 15:48:59 +0800
Subject: [PATCH] slub: reduce duplicate creation on the first object
When a kmem_cache is created with ctor, each object in the kmem_cache will be
initialized before ready to use. While in slub implementation, the first
object will be initialized twice.
This patch reduces the duplication of initialization of the first object.
Fix commit 7656c72b: SLUB: add macros for scanning objects in a slab.
Signed-off-by: Wei Yang <[email protected]>
---
mm/slub.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index b2b0473..79611d9 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -288,6 +288,10 @@ static inline void set_freepointer(struct kmem_cache *s, void *object, void *fp)
for (__p = (__addr); __p < (__addr) + (__objects) * (__s)->size;\
__p += (__s)->size)
+#define for_each_object_idx(__p, __idx, __s, __addr, __objects) \
+ for (__p = (__addr), __idx = 1; __idx <= __objects;\
+ __p += (__s)->size, __idx++)
+
/* Determine object index from a given position */
static inline int slab_index(void *p, struct kmem_cache *s, void *addr)
{
@@ -1409,9 +1413,9 @@ static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
{
struct page *page;
void *start;
- void *last;
void *p;
int order;
+ int idx;
BUG_ON(flags & GFP_SLAB_BUG_MASK);
@@ -1432,14 +1436,13 @@ static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
if (unlikely(s->flags & SLAB_POISON))
memset(start, POISON_INUSE, PAGE_SIZE << order);
- last = start;
- for_each_object(p, s, start, page->objects) {
- setup_object(s, page, last);
- set_freepointer(s, last, p);
- last = p;
+ for_each_object_idx(p, idx, s, start, page->objects) {
+ setup_object(s, page, p);
+ if (likely(idx < page->objects))
+ set_freepointer(s, p, p + s->size);
+ else
+ set_freepointer(s, p, NULL);
}
- setup_object(s, page, last);
- set_freepointer(s, last, NULL);
page->freelist = start;
page->inuse = page->objects;
--
1.7.9.5
--
Richard Yang
Help you, Help me
On Thu, 3 Jul 2014, Wei Yang wrote:
> Here is my refined version, hope this is more friendly to the audience.
Acked-by: Christoph Lameter <[email protected]>
On Mon, Jul 07, 2014 at 08:51:08AM -0500, Christoph Lameter wrote:
>On Thu, 3 Jul 2014, Wei Yang wrote:
>
>> Here is my refined version, hope this is more friendly to the audience.
>
>Acked-by: Christoph Lameter <[email protected]>
Thanks. I am glad to work with you.
--
Richard Yang
Help you, Help me