Messages in this thread | | | Subject | Re: [PATCH v3 1/2] mm: Provide empty function for kmem_dump_obj() when CONFIG_PRINTK=n | From | "Leizhen (ThunderTown)" <> | Date | Wed, 2 Aug 2023 19:11:04 +0800 |
| |
On 2023/8/2 11:57, Matthew Wilcox wrote: > On Wed, Aug 02, 2023 at 11:45:16AM +0800, thunder.leizhen@huaweicloud.com wrote: >> +++ b/include/linux/slab.h >> @@ -246,6 +246,9 @@ size_t ksize(const void *objp); >> #ifdef CONFIG_PRINTK >> bool kmem_valid_obj(void *object); >> void kmem_dump_obj(void *object); >> +#else >> +static inline bool kmem_valid_obj(void *object) { return false; } > > That is very confusing. kmem_valid_obj() looks like a function which > should exist regardless of CONFIG_PRINTK and to have it always return > false if CONFIG_PRINTK isn't set seems weird.
Yes, I noticed it, but I didn't come up with a good idea.
> > I see we have one caller of kmem_valid_obj() right now. Which means it > shouldn't be an EXPORT_SYMBOL since that caller is not a module. > > I think the right solution is to convert kmem_dump_obj() to > work the same way as vmalloc_dump_obj(). ie:
Okay, it's a good suggestion.
In fact, kmem_dump_obj() also does what kmem_valid_obj() does, except that it will print warning if the check fails. So, do as you suggest, the duplicated code can be eliminated.
> > +++ b/mm/util.c > @@ -1057,11 +1057,8 @@ void mem_dump_obj(void *object) > { > const char *type; > > - if (kmem_valid_obj(object)) { > - kmem_dump_obj(object); > + if (kmem_dump_obj(object)) > return; > - } > - > if (vmalloc_dump_obj(object)) > return; > > ... with corresponding changes to eliminate kmem_valid_obj() as a > symbol. > > . >
-- Regards, Zhen Lei
| |