* [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects()
@ 2018-10-25 9:44 Wei Yang
2018-10-25 9:44 ` [PATCH 2/3] mm, slub: unify access to s->cpu_slab by replacing raw_cpu_ptr() with this_cpu_ptr() Wei Yang
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Wei Yang @ 2018-10-25 9:44 UTC (permalink / raw)
To: cl, penberg, rientjes; +Cc: akpm, linux-mm, Wei Yang
In current code, the following context always meets:
local_irq_save/disable()
___slab_alloc()
new_slab_objects()
local_irq_restore/enable()
This context ensures cpu will continue running until it finish this job
before yield its control, which means the cpu_slab retrieved in
new_slab_objects() is the same as passed in.
Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
mm/slub.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index ce2b9e5cea77..11e49d95e0ac 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2402,10 +2402,9 @@ slab_out_of_memory(struct kmem_cache *s, gfp_t gfpflags, int nid)
}
static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags,
- int node, struct kmem_cache_cpu **pc)
+ int node, struct kmem_cache_cpu *c)
{
void *freelist;
- struct kmem_cache_cpu *c = *pc;
struct page *page;
WARN_ON_ONCE(s->ctor && (flags & __GFP_ZERO));
@@ -2417,7 +2416,6 @@ static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags,
page = new_slab(s, flags, node);
if (page) {
- c = raw_cpu_ptr(s->cpu_slab);
if (c->page)
flush_slab(s, c);
@@ -2430,7 +2428,6 @@ static inline void *new_slab_objects(struct kmem_cache *s, gfp_t flags,
stat(s, ALLOC_SLAB);
c->page = page;
- *pc = c;
} else
freelist = NULL;
@@ -2567,7 +2564,7 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
goto redo;
}
- freelist = new_slab_objects(s, gfpflags, node, &c);
+ freelist = new_slab_objects(s, gfpflags, node, c);
if (unlikely(!freelist)) {
slab_out_of_memory(s, gfpflags, node);
--
2.15.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] mm, slub: unify access to s->cpu_slab by replacing raw_cpu_ptr() with this_cpu_ptr()
2018-10-25 9:44 [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Wei Yang
@ 2018-10-25 9:44 ` Wei Yang
2018-10-25 13:53 ` Christopher Lameter
2018-10-25 9:44 ` [PATCH 3/3] mm, slub: make the comment of put_cpu_partial() complete Wei Yang
2018-10-25 13:46 ` [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Christopher Lameter
2 siblings, 1 reply; 10+ messages in thread
From: Wei Yang @ 2018-10-25 9:44 UTC (permalink / raw)
To: cl, penberg, rientjes; +Cc: akpm, linux-mm, Wei Yang
In current code, we use two forms to access s->cpu_slab
* raw_cpu_ptr()
* this_cpu_ptr()
This patch unify the access by replacing raw_cpu_ptr() with
this_cpu_ptr().
Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
mm/slub.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 11e49d95e0ac..715372a786e3 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2643,7 +2643,7 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
*/
do {
tid = this_cpu_read(s->cpu_slab->tid);
- c = raw_cpu_ptr(s->cpu_slab);
+ c = this_cpu_ptr(s->cpu_slab);
} while (IS_ENABLED(CONFIG_PREEMPT) &&
unlikely(tid != READ_ONCE(c->tid)));
@@ -2916,7 +2916,7 @@ static __always_inline void do_slab_free(struct kmem_cache *s,
*/
do {
tid = this_cpu_read(s->cpu_slab->tid);
- c = raw_cpu_ptr(s->cpu_slab);
+ c = this_cpu_ptr(s->cpu_slab);
} while (IS_ENABLED(CONFIG_PREEMPT) &&
unlikely(tid != READ_ONCE(c->tid)));
--
2.15.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] mm, slub: make the comment of put_cpu_partial() complete
2018-10-25 9:44 [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Wei Yang
2018-10-25 9:44 ` [PATCH 2/3] mm, slub: unify access to s->cpu_slab by replacing raw_cpu_ptr() with this_cpu_ptr() Wei Yang
@ 2018-10-25 9:44 ` Wei Yang
2018-10-25 13:41 ` Christopher Lameter
2018-10-25 13:46 ` [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Christopher Lameter
2 siblings, 1 reply; 10+ messages in thread
From: Wei Yang @ 2018-10-25 9:44 UTC (permalink / raw)
To: cl, penberg, rientjes; +Cc: akpm, linux-mm, Wei Yang
There are two cases when put_cpu_partial() is invoked.
* __slab_free
* get_partial_node
This patch just makes it cover these two cases and fix one typo in
slub_def.h.
Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
include/linux/slub_def.h | 2 +-
mm/slub.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index 3a1a1dbc6f49..201a635be846 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -81,7 +81,7 @@ struct kmem_cache_order_objects {
*/
struct kmem_cache {
struct kmem_cache_cpu __percpu *cpu_slab;
- /* Used for retriving partial slabs etc */
+ /* Used for retrieving partial slabs etc */
slab_flags_t flags;
unsigned long min_partial;
unsigned int size; /* The size of an object including meta data */
diff --git a/mm/slub.c b/mm/slub.c
index 715372a786e3..3db6ce58e92e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2201,8 +2201,8 @@ static void unfreeze_partials(struct kmem_cache *s,
}
/*
- * Put a page that was just frozen (in __slab_free) into a partial page
- * slot if available.
+ * Put a page that was just frozen (in __slab_free|get_partial_node) into a
+ * partial page slot if available.
*
* If we did not find a slot then simply move all the partials to the
* per node partial list.
--
2.15.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] mm, slub: make the comment of put_cpu_partial() complete
2018-10-25 9:44 ` [PATCH 3/3] mm, slub: make the comment of put_cpu_partial() complete Wei Yang
@ 2018-10-25 13:41 ` Christopher Lameter
2018-12-30 8:25 ` Wei Yang
0 siblings, 1 reply; 10+ messages in thread
From: Christopher Lameter @ 2018-10-25 13:41 UTC (permalink / raw)
To: Wei Yang; +Cc: penberg, rientjes, akpm, linux-mm
Acked-by: Christoph Lameter <cl@linux.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects()
2018-10-25 9:44 [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Wei Yang
2018-10-25 9:44 ` [PATCH 2/3] mm, slub: unify access to s->cpu_slab by replacing raw_cpu_ptr() with this_cpu_ptr() Wei Yang
2018-10-25 9:44 ` [PATCH 3/3] mm, slub: make the comment of put_cpu_partial() complete Wei Yang
@ 2018-10-25 13:46 ` Christopher Lameter
2018-10-25 14:54 ` Wei Yang
2018-10-26 4:33 ` Wei Yang
2 siblings, 2 replies; 10+ messages in thread
From: Christopher Lameter @ 2018-10-25 13:46 UTC (permalink / raw)
To: Wei Yang; +Cc: penberg, rientjes, akpm, linux-mm
On Thu, 25 Oct 2018, Wei Yang wrote:
> In current code, the following context always meets:
>
> local_irq_save/disable()
> ___slab_alloc()
> new_slab_objects()
> local_irq_restore/enable()
>
> This context ensures cpu will continue running until it finish this job
> before yield its control, which means the cpu_slab retrieved in
> new_slab_objects() is the same as passed in.
Interrupts can be switched on in new_slab() since it goes to the page
allocator. See allocate_slab().
This means that the percpu slab may change.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] mm, slub: unify access to s->cpu_slab by replacing raw_cpu_ptr() with this_cpu_ptr()
2018-10-25 9:44 ` [PATCH 2/3] mm, slub: unify access to s->cpu_slab by replacing raw_cpu_ptr() with this_cpu_ptr() Wei Yang
@ 2018-10-25 13:53 ` Christopher Lameter
2018-10-25 14:49 ` Wei Yang
0 siblings, 1 reply; 10+ messages in thread
From: Christopher Lameter @ 2018-10-25 13:53 UTC (permalink / raw)
To: Wei Yang; +Cc: penberg, rientjes, akpm, linux-mm
On Thu, 25 Oct 2018, Wei Yang wrote:
> In current code, we use two forms to access s->cpu_slab
>
> * raw_cpu_ptr()
> * this_cpu_ptr()
Ok the only difference is that for CONFIG_DEBUG_PREEMPT we will do the
debug checks twice.
That tolerable I think but is this really a worthwhile change?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] mm, slub: unify access to s->cpu_slab by replacing raw_cpu_ptr() with this_cpu_ptr()
2018-10-25 13:53 ` Christopher Lameter
@ 2018-10-25 14:49 ` Wei Yang
0 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2018-10-25 14:49 UTC (permalink / raw)
To: Christopher Lameter; +Cc: Wei Yang, penberg, rientjes, akpm, linux-mm
On Thu, Oct 25, 2018 at 01:53:06PM +0000, Christopher Lameter wrote:
>On Thu, 25 Oct 2018, Wei Yang wrote:
>
>> In current code, we use two forms to access s->cpu_slab
>>
>> * raw_cpu_ptr()
>> * this_cpu_ptr()
>
>Ok the only difference is that for CONFIG_DEBUG_PREEMPT we will do the
>debug checks twice.
>
>That tolerable I think but is this really a worthwhile change?
Agree.
My purpose is to make unify the access, looks easy for me to read the
code.
You can decide whether to change this or not :-)
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects()
2018-10-25 13:46 ` [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Christopher Lameter
@ 2018-10-25 14:54 ` Wei Yang
2018-10-26 4:33 ` Wei Yang
1 sibling, 0 replies; 10+ messages in thread
From: Wei Yang @ 2018-10-25 14:54 UTC (permalink / raw)
To: Christopher Lameter; +Cc: Wei Yang, penberg, rientjes, akpm, linux-mm
On Thu, Oct 25, 2018 at 01:46:49PM +0000, Christopher Lameter wrote:
>On Thu, 25 Oct 2018, Wei Yang wrote:
>
>> In current code, the following context always meets:
>>
>> local_irq_save/disable()
>> ___slab_alloc()
>> new_slab_objects()
>> local_irq_restore/enable()
>>
>> This context ensures cpu will continue running until it finish this job
>> before yield its control, which means the cpu_slab retrieved in
>> new_slab_objects() is the same as passed in.
>
>Interrupts can be switched on in new_slab() since it goes to the page
>allocator. See allocate_slab().
>
>This means that the percpu slab may change.
Ah, you are right, thank :-)
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects()
2018-10-25 13:46 ` [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Christopher Lameter
2018-10-25 14:54 ` Wei Yang
@ 2018-10-26 4:33 ` Wei Yang
1 sibling, 0 replies; 10+ messages in thread
From: Wei Yang @ 2018-10-26 4:33 UTC (permalink / raw)
To: Christopher Lameter; +Cc: Wei Yang, penberg, rientjes, akpm, linux-mm
Hi, Christopher
I got one confusion on understanding one case in __slab_free().
The case is : (new.frozen && !was_frozen)
My confusion is : Is it possible for the page to be on the full list?
This case (new.frozen && !was_frozen) happens when (!prior && !was_frozen).
* !prior means this page is full
* !was_frozen means this page is not in cpu_slab->page/partial
There are two cases to lead to (!prior && !was_frozen):
* in get_freelist(), when page is full
* in deactivate_slab(), when page is full
The first case will have a page in no list.
The second case will have a page in no list, or the page is put into
full list if SLUB_DEBUG is configured.
Do I miss something?
On Thu, Oct 25, 2018 at 01:46:49PM +0000, Christopher Lameter wrote:
>On Thu, 25 Oct 2018, Wei Yang wrote:
>
>> In current code, the following context always meets:
>>
>> local_irq_save/disable()
>> ___slab_alloc()
>> new_slab_objects()
>> local_irq_restore/enable()
>>
>> This context ensures cpu will continue running until it finish this job
>> before yield its control, which means the cpu_slab retrieved in
>> new_slab_objects() is the same as passed in.
>
>Interrupts can be switched on in new_slab() since it goes to the page
>allocator. See allocate_slab().
>
>This means that the percpu slab may change.
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] mm, slub: make the comment of put_cpu_partial() complete
2018-10-25 13:41 ` Christopher Lameter
@ 2018-12-30 8:25 ` Wei Yang
0 siblings, 0 replies; 10+ messages in thread
From: Wei Yang @ 2018-12-30 8:25 UTC (permalink / raw)
To: Christopher Lameter; +Cc: Wei Yang, penberg, rientjes, akpm, linux-mm
On Thu, Oct 25, 2018 at 01:41:58PM +0000, Christopher Lameter wrote:
>
>Acked-by: Christoph Lameter <cl@linux.com>
Andrew,
Do you like this one?
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-12-30 8:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 9:44 [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Wei Yang
2018-10-25 9:44 ` [PATCH 2/3] mm, slub: unify access to s->cpu_slab by replacing raw_cpu_ptr() with this_cpu_ptr() Wei Yang
2018-10-25 13:53 ` Christopher Lameter
2018-10-25 14:49 ` Wei Yang
2018-10-25 9:44 ` [PATCH 3/3] mm, slub: make the comment of put_cpu_partial() complete Wei Yang
2018-10-25 13:41 ` Christopher Lameter
2018-12-30 8:25 ` Wei Yang
2018-10-25 13:46 ` [PATCH 1/3] mm, slub: not retrieve cpu_slub again in new_slab_objects() Christopher Lameter
2018-10-25 14:54 ` Wei Yang
2018-10-26 4:33 ` Wei Yang
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).