| Message ID | 20210520013539.3733631-4-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | slub: Print non-hashed pointers in slub debugging | expand |
On 5/20/21 3:35 AM, Stephen Boyd wrote: > The message argument isn't used here. Let's pass the string to the > printk message so that the developer can figure out what's happening, > instead of guessing that a redzone is being restored, etc. > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/slub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 87eeeed1f369..16e8e8f8dc0c 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -777,7 +777,7 @@ static void init_object(struct kmem_cache *s, void *object, u8 val) > static void restore_bytes(struct kmem_cache *s, char *message, u8 data, > void *from, void *to) > { > - slab_fix(s, "Restoring 0x%px-0x%px=0x%x\n", from, to - 1, data); > + slab_fix(s, "Restoring %s 0x%px-0x%px=0x%x\n", message, from, to - 1, data); > memset(from, data, to - from); > } > >
On Wed, 19 May 2021, Stephen Boyd wrote: > The message argument isn't used here. Let's pass the string to the > printk message so that the developer can figure out what's happening, > instead of guessing that a redzone is being restored, etc. > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> Acked-by: David Rientjes <rientjes@google.com>
On Sun, 2021-05-23 at 22:12 -0700, David Rientjes wrote: > On Wed, 19 May 2021, Stephen Boyd wrote: > > > The message argument isn't used here. Let's pass the string to the > > printk message so that the developer can figure out what's happening, > > instead of guessing that a redzone is being restored, etc. > > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > Acked-by: David Rientjes <rientjes@google.com> Ideally, the slab_fix function would be marked with __printf and the format here would not use \n as that's emitted by the slab_fix. --- mm/slub.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index ee51857d8e9bc..46f9b043089b6 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -702,6 +702,7 @@ static void slab_bug(struct kmem_cache *s, char *fmt, ...) va_end(args); } +__printf(2, 3) static void slab_fix(struct kmem_cache *s, char *fmt, ...) { struct va_format vaf; @@ -816,7 +817,8 @@ static void init_object(struct kmem_cache *s, void *object, u8 val) static void restore_bytes(struct kmem_cache *s, char *message, u8 data, void *from, void *to) { - slab_fix(s, "Restoring %s 0x%px-0x%px=0x%x\n", message, from, to - 1, data); + slab_fix(s, "Restoring %s 0x%px-0x%px=0x%x", + message, from, to - 1, data); memset(from, data, to - from); } @@ -1069,13 +1071,13 @@ static int on_freelist(struct kmem_cache *s, struct page *page, void *search) slab_err(s, page, "Wrong number of objects. Found %d but should be %d", page->objects, max_objects); page->objects = max_objects; - slab_fix(s, "Number of objects adjusted."); + slab_fix(s, "Number of objects adjusted"); } if (page->inuse != page->objects - nr) { slab_err(s, page, "Wrong object count. Counter is %d but counted were %d", page->inuse, page->objects - nr); page->inuse = page->objects - nr; - slab_fix(s, "Object count adjusted."); + slab_fix(s, "Object count adjusted"); } return search == NULL; }
Quoting Joe Perches (2021-05-25 00:37:45) > On Sun, 2021-05-23 at 22:12 -0700, David Rientjes wrote: > > On Wed, 19 May 2021, Stephen Boyd wrote: > > > > > The message argument isn't used here. Let's pass the string to the > > > printk message so that the developer can figure out what's happening, > > > instead of guessing that a redzone is being restored, etc. > > > > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > > > Acked-by: David Rientjes <rientjes@google.com> > > Ideally, the slab_fix function would be marked with __printf and the > format here would not use \n as that's emitted by the slab_fix. Thanks. I can make this into a proper patch and author it from you. Can you provide a signed-off-by? The restore_bytes() hunk is slightly different but I can fix that up. > --- > mm/slub.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index ee51857d8e9bc..46f9b043089b6 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -702,6 +702,7 @@ static void slab_bug(struct kmem_cache *s, char *fmt, ...) > va_end(args); > } > > +__printf(2, 3) > static void slab_fix(struct kmem_cache *s, char *fmt, ...) > { > struct va_format vaf; > @@ -816,7 +817,8 @@ static void init_object(struct kmem_cache *s, void *object, u8 val) > static void restore_bytes(struct kmem_cache *s, char *message, u8 data, > void *from, void *to) > { > - slab_fix(s, "Restoring %s 0x%px-0x%px=0x%x\n", message, from, to - 1, data); > + slab_fix(s, "Restoring %s 0x%px-0x%px=0x%x", > + message, from, to - 1, data); > memset(from, data, to - from); > } > > @@ -1069,13 +1071,13 @@ static int on_freelist(struct kmem_cache *s, struct page *page, void *search) > slab_err(s, page, "Wrong number of objects. Found %d but should be %d", > page->objects, max_objects); > page->objects = max_objects; > - slab_fix(s, "Number of objects adjusted."); > + slab_fix(s, "Number of objects adjusted"); > } > if (page->inuse != page->objects - nr) { > slab_err(s, page, "Wrong object count. Counter is %d but counted were %d", > page->inuse, page->objects - nr); > page->inuse = page->objects - nr; > - slab_fix(s, "Object count adjusted."); > + slab_fix(s, "Object count adjusted"); > } > return search == NULL; > } > >
On Tue, 2021-05-25 at 22:32 -0400, Stephen Boyd wrote: > Quoting Joe Perches (2021-05-25 00:37:45) > > On Sun, 2021-05-23 at 22:12 -0700, David Rientjes wrote: > > > On Wed, 19 May 2021, Stephen Boyd wrote: > > > > > > > The message argument isn't used here. Let's pass the string to the > > > > printk message so that the developer can figure out what's happening, > > > > instead of guessing that a redzone is being restored, etc. > > > > > > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > > > > > Acked-by: David Rientjes <rientjes@google.com> > > > > Ideally, the slab_fix function would be marked with __printf and the > > format here would not use \n as that's emitted by the slab_fix. > > Thanks. I can make this into a proper patch and author it from you. Can > you provide a signed-off-by? The restore_bytes() hunk is slightly > different but I can fix that up. If you want... Signed-off-by: Joe Perches <joe@perches.com> > > > --- > > mm/slub.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/mm/slub.c b/mm/slub.c > > index ee51857d8e9bc..46f9b043089b6 100644 > > --- a/mm/slub.c > > +++ b/mm/slub.c > > @@ -702,6 +702,7 @@ static void slab_bug(struct kmem_cache *s, char *fmt, ...) > > va_end(args); > > } > > > > +__printf(2, 3) > > static void slab_fix(struct kmem_cache *s, char *fmt, ...) > > { > > struct va_format vaf; > > @@ -816,7 +817,8 @@ static void init_object(struct kmem_cache *s, void *object, u8 val) > > static void restore_bytes(struct kmem_cache *s, char *message, u8 data, > > void *from, void *to) > > { > > - slab_fix(s, "Restoring %s 0x%px-0x%px=0x%x\n", message, from, to - 1, data); > > + slab_fix(s, "Restoring %s 0x%px-0x%px=0x%x", > > + message, from, to - 1, data); > > memset(from, data, to - from); > > } > > > > @@ -1069,13 +1071,13 @@ static int on_freelist(struct kmem_cache *s, struct page *page, void *search) > > slab_err(s, page, "Wrong number of objects. Found %d but should be %d", > > page->objects, max_objects); > > page->objects = max_objects; > > - slab_fix(s, "Number of objects adjusted."); > > + slab_fix(s, "Number of objects adjusted"); > > } > > if (page->inuse != page->objects - nr) { > > slab_err(s, page, "Wrong object count. Counter is %d but counted were %d", > > page->inuse, page->objects - nr); > > page->inuse = page->objects - nr; > > - slab_fix(s, "Object count adjusted."); > > + slab_fix(s, "Object count adjusted"); > > } > > return search == NULL; > > } > > > >
diff --git a/mm/slub.c b/mm/slub.c index 87eeeed1f369..16e8e8f8dc0c 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -777,7 +777,7 @@ static void init_object(struct kmem_cache *s, void *object, u8 val) static void restore_bytes(struct kmem_cache *s, char *message, u8 data, void *from, void *to) { - slab_fix(s, "Restoring 0x%px-0x%px=0x%x\n", from, to - 1, data); + slab_fix(s, "Restoring %s 0x%px-0x%px=0x%x\n", message, from, to - 1, data); memset(from, data, to - from); }
The message argument isn't used here. Let's pass the string to the printk message so that the developer can figure out what's happening, instead of guessing that a redzone is being restored, etc. Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- mm/slub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)