[Crash-utility] Re: [PATCH] kmem: introduce -t flag to get page owner or slab debug trace
Hi Tao,
Thanks for your review.
This is a big patch, can we separate it into multiple smaller ones?
From my understanding about this patch, it involved 2 main features,
one for dump the page_owner allocated stack trace, one for dump the
slab debug trace. So at least it can be divided into 2 from my view.
Sure, I will divided it into 2 changes and resend them for review.
Why put the tflag check within the pflag check? Do you think the
following would be better?
if (tflag)
meminfo.flags |= GET_PAGE_OWNER;
if (fplag) {...}
because the -t flag is introduced for only two usage:
+#define GET_SLAB_DEBUG_TRACE (ADDRESS_SPECIFIED << 28)
+#define GET_PAGE_OWNER (ADDRESS_SPECIFIED << 29)
so the extra tflag must be used with -p and -S or -s.
if (tflag)
meminfo.flags = GET_PAGE_OWNER;
if (pflag == 1) {...}
This variable is not used, can you check its usage?
Here is used with "kmem -pt" without specified address to get page_owner
for all allocated page in buddy system.
Also for this large inserting hunk, please reformat your patch, keep
the length of code no longer than 80 chars.
There is no need to write a new function for it, we can reuse the
existing function get_uptime().
better put the check into the caller:
if (si->flags & GET_SLAB_DEBUG_TRACE)
slab_debug_trace_show();
Okay, I will make an improvement in patch V2 as your
suggestions.
Thanks
Qiwu