| Message ID | 20210421022518.67451-1-yejune.deng@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/slab.c: use 'ac' from the caller | expand |
On Wed, Apr 21, 2021 at 10:25:17AM +0800, Yejune Deng wrote: > @@ -3045,12 +3044,7 @@ static inline void *____cache_alloc(struct kmem_cache *cachep, gfp_t flags) > } > > STATS_INC_ALLOCMISS(cachep); > - objp = cache_alloc_refill(cachep, flags); > - /* > - * the 'ac' may be updated by cache_alloc_refill(), > - * and kmemleak_erase() requires its correct value. > - */ > - ac = cpu_cache_get(cachep); > + objp = cache_alloc_refill(cachep, ac, flags); I think passing 'ac' in is fine (probably? I don't know this code deeply), but deleting this call to 'ac' is clearly wrong. The comment even tells you that! I just verified the code, and the comment is correct.
On Wed, 21 Apr 2021, Matthew Wilcox wrote: > On Wed, Apr 21, 2021 at 10:25:17AM +0800, Yejune Deng wrote: > > @@ -3045,12 +3044,7 @@ static inline void *____cache_alloc(struct kmem_cache *cachep, gfp_t flags) > > } > > > > STATS_INC_ALLOCMISS(cachep); > > - objp = cache_alloc_refill(cachep, flags); > > - /* > > - * the 'ac' may be updated by cache_alloc_refill(), > > - * and kmemleak_erase() requires its correct value. > > - */ > > - ac = cpu_cache_get(cachep); > > + objp = cache_alloc_refill(cachep, ac, flags); > > I think passing 'ac' in is fine (probably? I don't know this code > deeply), but deleting this call to 'ac' is clearly wrong. The comment > even tells you that! I just verified the code, and the comment is > correct. Yep the delete of the ac assignment is wrong. But even without that issue: There is no point to passing ac to cache_alloc_refill since cpu_cache_get is rather trivial and does not even require memory access since "cachep" is usually in some register.
diff --git a/mm/slab.c b/mm/slab.c index d0f725637663..4b2dc8f8cc37 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -2896,11 +2896,11 @@ static __always_inline int alloc_block(struct kmem_cache *cachep, return batchcount; } -static void *cache_alloc_refill(struct kmem_cache *cachep, gfp_t flags) +static void *cache_alloc_refill(struct kmem_cache *cachep, struct array_cache *ac, gfp_t flags) { int batchcount; struct kmem_cache_node *n; - struct array_cache *ac, *shared; + struct array_cache *shared; int node; void *list = NULL; struct page *page; @@ -2908,7 +2908,6 @@ static void *cache_alloc_refill(struct kmem_cache *cachep, gfp_t flags) check_irq_off(); node = numa_mem_id(); - ac = cpu_cache_get(cachep); batchcount = ac->batchcount; if (!ac->touched && batchcount > BATCHREFILL_LIMIT) { /* @@ -3045,12 +3044,7 @@ static inline void *____cache_alloc(struct kmem_cache *cachep, gfp_t flags) } STATS_INC_ALLOCMISS(cachep); - objp = cache_alloc_refill(cachep, flags); - /* - * the 'ac' may be updated by cache_alloc_refill(), - * and kmemleak_erase() requires its correct value. - */ - ac = cpu_cache_get(cachep); + objp = cache_alloc_refill(cachep, ac, flags); out: /*
It can use 'ac' from ____cache_alloc() in cache_alloc_refill(). This saves call cpu_cache_get() twice. Signed-off-by: Yejune Deng <yejune.deng@gmail.com> --- mm/slab.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)