Skip to contentSkip to conversation

sh: Update kmem_cache_flag.

PATCH In review Yoshinori Sato 2 participants 4 messages Aug 2, 2018 SuperH

lore
Share & tools
lore

Yoshinori Sato

author maintainer
#0 lore raw

PATCH v2 1/1 sh: Update kmem_cache_flag.

__GFP_ZERO and the constructor are exclusive relationships, so it is incorrect to specify them at the same time.

---
 arch/sh/mm/pgtable.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/sh/mm/pgtable.c +4 -4
index 5c8f9247c3c2..5a54c2bce2de 100644
@@ -2,8 +2,6 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 
-#define PGALLOC_GFP GFP_KERNEL | __GFP_ZERO
-
 static struct kmem_cache *pgd_cachep;
 #if PAGETABLE_LEVELS > 2
 static struct kmem_cache *pmd_cachep;

@@ -32,7 +30,9 @@ void pgtable_cache_init(void)
 
 pgd_t *pgd_alloc(struct mm_struct *mm)
 {
-	return kmem_cache_alloc(pgd_cachep, PGALLOC_GFP);
+	pgd_t *pgd = kmem_cache_alloc(pgd_cachep, GFP_KERNEL);
+	memset(pgd, 0, kmem_cache_size(pgd_cachep));
+	return pgd;
 }
 
 void pgd_free(struct mm_struct *mm, pgd_t *pgd)

@@ -48,7 +48,7 @@ void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
 
 pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
 {
-	return kmem_cache_alloc(pmd_cachep, PGALLOC_GFP);
+	return kmem_cache_alloc(pmd_cachep, GFP_KERNEL | __GFP_ZERO);
 }
 
 void pmd_free(struct mm_struct *mm, pmd_t *pmd)
Signature · 1 line
2.11.0
#1 lore raw

Hi Sato-san,

Quoted 2 lines · Yoshinori Sato

__GFP_ZERO and the constructor are exclusive relationships, so it is incorrect to specify them at the same time.

Thanks for your patch!

Quoted 12 lines
arch/sh/mm/pgtable.c +0 -2
@@ -2,8 +2,6 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 
-#define PGALLOC_GFP GFP_KERNEL | __GFP_ZERO
-
 static struct kmem_cache *pgd_cachep;
 #if PAGETABLE_LEVELS > 2
 static struct kmem_cache *pmd_cachep;

arch/sh/mm/pgtable.c void pgtable_cache_init(void) · line 34

Quoted 6 lines
arch/sh/mm/pgtable.c +2 -1
@@ -32,7 +30,9 @@ void pgtable_cache_init(void)
 
 pgd_t *pgd_alloc(struct mm_struct *mm)
 {
-       return kmem_cache_alloc(pgd_cachep, PGALLOC_GFP);
+       pgd_t *pgd = kmem_cache_alloc(pgd_cachep, GFP_KERNEL);
+       memset(pgd, 0, kmem_cache_size(pgd_cachep));

Are you sure the above line is needed?
As kmem_cache_size(pgd_cachep) = 4096, this clears the whole pgd again, undoing the constructor's work.

arch/sh/mm/pgtable.c void pgtable_cache_init(void) · line 38

Quoted 4 lines
arch/sh/mm/pgtable.c +1 -0
+       return pgd;
 }
 
 void pgd_free(struct mm_struct *mm, pgd_t *pgd)

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
author maintainer
#2 lore raw
In reply to Yoshinori Sato
Quoted 23 lines · Geert Uytterhoeven

Hi Sato-san,

Quoted 2 lines · Yoshinori Sato

__GFP_ZERO and the constructor are exclusive relationships, so it is incorrect to specify them at the same time.

Thanks for your patch!

Quoted 13 lines
arch/sh/mm/pgtable.c +0 -2
@@ -2,8 +2,6 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 
-#define PGALLOC_GFP GFP_KERNEL | __GFP_ZERO
-
 static struct kmem_cache *pgd_cachep;
 #if PAGETABLE_LEVELS > 2
 static struct kmem_cache *pmd_cachep;

@@ -32,7 +30,9 @@ void pgtable_cache_init(void)
Quoted 9 lines

arch/sh/mm/pgtable.c void pgtable_cache_init(void) · line 34

Quoted 5 lines
arch/sh/mm/pgtable.c +2 -1
 pgd_t *pgd_alloc(struct mm_struct *mm)
 {
-       return kmem_cache_alloc(pgd_cachep, PGALLOC_GFP);
+       pgd_t *pgd = kmem_cache_alloc(pgd_cachep, GFP_KERNEL);
+       memset(pgd, 0, kmem_cache_size(pgd_cachep));

Are you sure the above line is needed?
As kmem_cache_size(pgd_cachep) = 4096, this clears the whole pgd again, undoing the constructor's work.

If there is no such line, panic with page fault.
I have not identified the cause, but there seems
to be a part that expects 0 somewhere.

Quoted 15 lines

arch/sh/mm/pgtable.c void pgtable_cache_init(void) · line 38

Quoted 4 lines
arch/sh/mm/pgtable.c +1 -0
+       return pgd;
 }
 
 void pgd_free(struct mm_struct *mm, pgd_t *pgd)

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Signature · 1 line
Yosinori Sato
#3 lore raw
In reply to Yoshinori Sato

Hi Sato-san,

Quoted 32 lines · Yoshinori Sato
Quoted 20 lines · Geert Uytterhoeven
Quoted 2 lines · Yoshinori Sato

__GFP_ZERO and the constructor are exclusive relationships, so it is incorrect to specify them at the same time.

Thanks for your patch!

Quoted 13 lines
arch/sh/mm/pgtable.c +0 -2
@@ -2,8 +2,6 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 
-#define PGALLOC_GFP GFP_KERNEL | __GFP_ZERO
-
 static struct kmem_cache *pgd_cachep;
 #if PAGETABLE_LEVELS > 2
 static struct kmem_cache *pmd_cachep;

@@ -32,7 +30,9 @@ void pgtable_cache_init(void)
Quoted 9 lines

arch/sh/mm/pgtable.c void pgtable_cache_init(void) · line 34

Quoted 5 lines
arch/sh/mm/pgtable.c +2 -1
 pgd_t *pgd_alloc(struct mm_struct *mm)
 {
-       return kmem_cache_alloc(pgd_cachep, PGALLOC_GFP);
+       pgd_t *pgd = kmem_cache_alloc(pgd_cachep, GFP_KERNEL);
+       memset(pgd, 0, kmem_cache_size(pgd_cachep));

Are you sure the above line is needed?
As kmem_cache_size(pgd_cachep) = 4096, this clears the whole pgd again, undoing the constructor's work.

Quoted 3 lines

If there is no such line, panic with page fault.
I have not identified the cause, but there seems
to be a part that expects 0 somewhere.

On QEMU emulating rts7751r2d it works fine.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds