* [PATCH] Tracepoint Tapset for Memory Subsystem @ 2009-09-19 5:01 Rajasekhar Duddu 2009-09-22 17:39 ` David Smith 0 siblings, 1 reply; 18+ messages in thread From: Rajasekhar Duddu @ 2009-09-19 5:01 UTC (permalink / raw) To: systemtap Hi all, here I am posting a patch for Memory Tapset based on kernel Tracepoints. This patch adds tracepoint tapset to memory.stp and a testcase (mem_tracepoints.stp) which tests if all probes in the tapset are resolvable . Signed-off-by: Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> diff -rupaN a/tapset/memory.stp b/tapset/memory.stp --- a/tapset/memory.stp 2009-09-16 14:24:21.000000000 +0200 +++ b/tapset/memory.stp 2009-09-19 06:33:34.000000000 +0200 @@ -195,3 +195,132 @@ probe vm.brk = kernel.function("do_brk") probe vm.oom_kill = kernel.function("__oom_kill_task") { task = $p } + + +/*Function to convert the GFP_FLAGS .*/ +function gfp_f:string (gfp_flag:long) +%{ + switch (THIS->gfp_flag) { + case 32: + strlcpy(THIS->__retvalue, "GFP_ATOMIC", MAXSTRINGLEN); + break; + case 208: + strlcpy(THIS->__retvalue, "GFP_KERNEL", MAXSTRINGLEN); + break; + case 16: + strlcpy(THIS->__retvalue, "GFP_NOIO", MAXSTRINGLEN); + break; + case 80: + strlcpy(THIS->__retvalue, "GFP_NOFS", MAXSTRINGLEN); + break; + case 524496: + strlcpy(THIS->__retvalue, "GFP_TEMPORARY", MAXSTRINGLEN); + break; + case 8400: + strlcpy(THIS->__retvalue, "GFP_USER", MAXSTRINGLEN); + break; + case 8402: + strlcpy(THIS->__retvalue, "GFP_HIGHUSER", MAXSTRINGLEN); + break; + case 8410: + strlcpy(THIS->__retvalue, "GFP_HIGHUSER_MOVABLE", MAXSTRINGLEN); + break; + default: + break; + } + +%} + +/** + * probe mem.kmalloc - Fires when <command>kmalloc</command> is requested. + * @call_site: Address of the memory function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: type of memory to allocate + * @ptr: Pointer to the memory allocated + */ + +probe mem.kmalloc = kernel.trace("kmalloc") { + name = "kmalloc" + call_site = symname($call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_f($gfp_flags) + ptr = $ptr +} + +/** + * probe mem.kmem_cache_alloc - Fires when \ + * <command>kmem_cache_alloc</command> is requested + * @call_site: Address of the function calling this memory function + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of memory to allocate + * @ptr: Pointer to the memory allocated + */ +probe mem.kmem_cache_alloc = kernel.trace("kmem_cache_alloc") { + name = "kmem_cache_alloc" + call_site = symname($call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_f($gfp_flags) + ptr = $ptr +} + +/** + * probe mem.kmalloc_node - Fires when <command>kmalloc_node</command> is requested + * @call_site: Address of the function caling this memory function + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of memory to allocate + * @ptr: Pointer to the memory allocated + */ +probe mem.kmalloc_node = kernel.trace("kmalloc_node")? { + name = "kmalloc_node" + call_site = symname($call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_f($gfp_flags) + ptr = $ptr +} + +/** + * probe mem.kmem_cache_alloc_node - Fires when \ + * <command>kmem_cache_alloc_node</command> is requested + * @call_site: Address of the function calling this memory function + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of memory to allocate + * @ptr: Pointer to the memory allocated + */ +probe mem.kmem_cache_alloc_node = kernel.trace("kmem_cache_alloc_node")? { + name = "kmem_cache_alloc_node" + call_site = symname($call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags =gfp_f($gfp_flags) + ptr = $ptr +} + +/** + * probe mem.kfree - Fires when <command>kfree</comand> is requested. + * @call_site: Address of the function calling this memory function. + * @ptr: Pointer to the memory allocated which is returned by kmalloc + */ +probe mem.kfree = kernel.trace("kfree") { + name = "kfree" + call_site = symname($call_site) + ptr = $ptr +} + +/** + * probe mem.kmem_cache_free - Fires when \ + * <command>kmem_cache_free</command> is requested. + * @call_site: Address of the function calling this memory function. + * @ptr: Pointer to the memory allocated which is returned by kmem_cache + */ +probe mem.kmem_cache_free = kernel.trace("kmem_cache_free") { + name = "kmem_cache_free" + call_site = symname($call_site) + ptr = $ptr +} diff -rupaN a/testsuite/buildok/mem_tracepoints.stp b/testsuite/buildok/mem_tracepoints.stp --- a/testsuite/buildok/mem_tracepoints.stp 1970-01-01 01:00:00.000000000 +0100 +++ b/testsuite/buildok/mem_tracepoints.stp 2009-09-19 06:25:06.000000000 +0200 @@ -0,0 +1,35 @@ +#!/usr/bin/stp -up4 +probe mem.kfree { + println (name) + printf("%-15s %-15s %-15p \n", execname(),call_site,ptr) +} + +probe mem.kmalloc { + println (name) + printf("%-15s %-15s %-15p %-15d %-15d %-15s \n", execname(),call_site,ptr,bytes_req,bytes_alloc,gfp_flags) +} + +probe mem.kmem_cache_alloc { + println (name) + printf("%-15s %-15s %-15p %-15d %-15d %-15s \n", execname(),call_site,ptr,bytes_req,bytes_alloc,gfp_flags) +} +probe mem.kmalloc_node { + println (name) + printf("%-15s %-15s %-15p %-15d %-15d %-15s \n", execname(),call_site,ptr,bytes_req,bytes_alloc,gfp_flags) +} + +probe mem.kmem_cache_alloc_node { + println (name) + printf("%-15s %-15s %-15p %-15d %-15d %-15s \n", execname(),call_site,ptr,bytes_req,bytes_alloc,gfp_flags) +} + +probe mem.kmem_cache_free { + println (name) + printf("%-15s %-15s %-15p \n", execname(),call_site,ptr) +} + +probe timer.s(1) { + exit() +} + + Thanks -- Rajasekhar Duddu (rajduddu@linux.vnet.ibm.com), Linux on System z - CSVT, IBM LTC, Bangalore. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Tracepoint Tapset for Memory Subsystem 2009-09-19 5:01 [PATCH] Tracepoint Tapset for Memory Subsystem Rajasekhar Duddu @ 2009-09-22 17:39 ` David Smith 2009-09-22 21:23 ` Frank Ch. Eigler 0 siblings, 1 reply; 18+ messages in thread From: David Smith @ 2009-09-22 17:39 UTC (permalink / raw) To: Rajasekhar Duddu; +Cc: systemtap On 09/19/2009 12:01 AM, Rajasekhar Duddu wrote: > Hi all, > here I am posting a patch for Memory Tapset based on > kernel Tracepoints. > > This patch adds tracepoint tapset to memory.stp and a testcase > (mem_tracepoints.stp) which tests if all probes in the > tapset are resolvable . In general this looks reasonable, and thanks for including the testcase. One comment below. > > > Signed-off-by: Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> > diff -rupaN a/tapset/memory.stp b/tapset/memory.stp > --- a/tapset/memory.stp 2009-09-16 14:24:21.000000000 +0200 > +++ b/tapset/memory.stp 2009-09-19 06:33:34.000000000 +0200 > @@ -195,3 +195,132 @@ probe vm.brk = kernel.function("do_brk") > probe vm.oom_kill = kernel.function("__oom_kill_task") { > task = $p > } > + > + > +/*Function to convert the GFP_FLAGS .*/ > +function gfp_f:string (gfp_flag:long) > +%{ > + switch (THIS->gfp_flag) { > + case 32: > + strlcpy(THIS->__retvalue, "GFP_ATOMIC", MAXSTRINGLEN); > + break; > + case 208: > + strlcpy(THIS->__retvalue, "GFP_KERNEL", MAXSTRINGLEN); > + break; > + case 16: > + strlcpy(THIS->__retvalue, "GFP_NOIO", MAXSTRINGLEN); > + break; > + case 80: > + strlcpy(THIS->__retvalue, "GFP_NOFS", MAXSTRINGLEN); > + break; > + case 524496: > + strlcpy(THIS->__retvalue, "GFP_TEMPORARY", MAXSTRINGLEN); > + break; > + case 8400: > + strlcpy(THIS->__retvalue, "GFP_USER", MAXSTRINGLEN); > + break; > + case 8402: > + strlcpy(THIS->__retvalue, "GFP_HIGHUSER", MAXSTRINGLEN); > + break; > + case 8410: > + strlcpy(THIS->__retvalue, "GFP_HIGHUSER_MOVABLE", MAXSTRINGLEN); > + break; > + default: > + break; > + } > + > +%} Hmm. Have you tried something like this instead (to avoid the hardcoded constants)? Note that I haven't actually tried this... ==== %{ #include <linux/gfp.h> %} function gfp_f:string (gfp_flag:long) %{ switch (THIS->gfp_flag) { case GFP_ATOMIC: strlcpy(THIS->__retvalue, "GFP_ATOMIC", MAXSTRINGLEN); break; case GFP_KERNEL: strlcpy(THIS->__retvalue, "GFP_KERNEL", MAXSTRINGLEN); break; ... %} ==== -- David Smith dsmith@redhat.com Red Hat http://www.redhat.com 256.217.0141 (direct) 256.837.0057 (fax) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Tracepoint Tapset for Memory Subsystem 2009-09-22 17:39 ` David Smith @ 2009-09-22 21:23 ` Frank Ch. Eigler 2009-09-22 22:05 ` David Smith 0 siblings, 1 reply; 18+ messages in thread From: Frank Ch. Eigler @ 2009-09-22 21:23 UTC (permalink / raw) To: David Smith; +Cc: Rajasekhar Duddu, systemtap David Smith <dsmith@redhat.com> writes: > [...] > function gfp_f:string (gfp_flag:long) > %{ > switch (THIS->gfp_flag) { > case GFP_ATOMIC: > strlcpy(THIS->__retvalue, "GFP_ATOMIC", MAXSTRINGLEN); > break; > case GFP_KERNEL: > strlcpy(THIS->__retvalue, "GFP_KERNEL", MAXSTRINGLEN); > break; > %} Actually, those values are bit masks, so might as well be treated like the open flags in syscall*.stp. - FChE ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] Tracepoint Tapset for Memory Subsystem 2009-09-22 21:23 ` Frank Ch. Eigler @ 2009-09-22 22:05 ` David Smith 2009-09-24 18:08 ` [PATCH v2] " Rajasekhar Duddu 0 siblings, 1 reply; 18+ messages in thread From: David Smith @ 2009-09-22 22:05 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: Rajasekhar Duddu, systemtap On 09/22/2009 04:23 PM, Frank Ch. Eigler wrote: > David Smith <dsmith@redhat.com> writes: > >> [...] >> function gfp_f:string (gfp_flag:long) >> %{ >> switch (THIS->gfp_flag) { >> case GFP_ATOMIC: >> strlcpy(THIS->__retvalue, "GFP_ATOMIC", MAXSTRINGLEN); >> break; >> case GFP_KERNEL: >> strlcpy(THIS->__retvalue, "GFP_KERNEL", MAXSTRINGLEN); >> break; >> %} > > Actually, those values are bit masks, so might as well be treated like > the open flags in syscall*.stp. While they are bitmasks, I'd much rather see 'GFP_KERNEL' than '(__GFP_WAIT|__GFP_IO|__GFP_FS)'. -- David Smith dsmith@redhat.com Red Hat http://www.redhat.com 256.217.0141 (direct) 256.837.0057 (fax) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Tracepoint Tapset for Memory Subsystem 2009-09-22 22:05 ` David Smith @ 2009-09-24 18:08 ` Rajasekhar Duddu 2009-09-25 19:19 ` David Smith 2009-09-25 21:50 ` Josh Stone 0 siblings, 2 replies; 18+ messages in thread From: Rajasekhar Duddu @ 2009-09-24 18:08 UTC (permalink / raw) To: systemtap Hi, I have modified the patch according to the comments passed by Frank and David. Many thanks to Frank And David. Changelog: Removed the Hardcoded constants in converting GFPFLAGS. Added a kprobe based fallback probe to kfree. Signed-off-by: Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> diff -rupaN a/tapset/memory.stp b/tapset/memory.stp --- a/tapset/memory.stp 2009-09-24 19:01:10.000000000 +0200 +++ b/tapset/memory.stp 2009-09-24 19:13:30.000000000 +0200 @@ -195,3 +195,205 @@ probe vm.brk = kernel.function("do_brk") probe vm.oom_kill = kernel.function("__oom_kill_task") { task = $p } +#Function to get the call_site for kprobe based kfree probe. + +function call_site:long () +%{ + int *ptr ; + ptr = __builtin_return_address(0); + THIS->__retvalue = *ptr; +%} + + +/*Function to convert the GFP_FLAGS .*/ +function gfp_f:string (gfp_flag:long) +%{ +int flags = (int)THIS->gfp_flag; + + +#ifdef __GFP_HIGH + if (flags & __GFP_HIGH) + strlcat (THIS->__retvalue, "GFP_HIGH",MAXSTRINGLEN); +#endif + +#ifdef __GFP_WAIT + if (flags & __GFP_WAIT) + strlcat (THIS->__retvalue, "GFP_WAIT",MAXSTRINGLEN); +#endif + +#ifdef __GFP_IO + if (flags & __GFP_IO) + strlcat (THIS->__retvalue, "|GFP_IO",MAXSTRINGLEN); +#endif + +#ifdef __GFP_FS + if (flags & __GFP_FS) + strlcat (THIS->__retvalue, "|GFP_FS",MAXSTRINGLEN); +#endif + +#ifdef __GFP_COLD + if (flags & __GFP_COLD) + strlcat (THIS->__retvalue, "|GFP_COLD",MAXSTRINGLEN); +#endif +#ifdef __GFP_NOWARN + if (flags & __GFP_NOWARN) + strlcat (THIS->__retvalue, "|GFP_NOWARN",MAXSTRINGLEN); +#endif + +#ifdef __GFP_REPEAT + if (flags & __GFP_REPEAT) + strlcat (THIS->__retvalue, "|GFP_REPEAT",MAXSTRINGLEN); +#endif + +#ifdef __GFP_NOFAIL + if (flags & __GFP_NOFAIL) + strlcat (THIS->__retvalue, "|GFP_NOFAIL",MAXSTRINGLEN); +#endif + +#ifdef __GFP_NORETRY + if (flags & __GFP_NORETRY) + strlcat (THIS->__retvalue, "|GFP_NORETRY",MAXSTRINGLEN); +#endif + +#ifdef __GFP_COMP + if (flags & __GFP_COMP) + strlcat (THIS->__retvalue, "|GFP_COMP",MAXSTRINGLEN); +#endif + +#ifdef __GFP_ZERO + if (flags & __GFP_ZERO) + strlcat (THIS->__retvalue, "|GFP_ZERO",MAXSTRINGLEN); +#endif + +#ifdef __GFP_NOMEMALLOC + if (flags & __GFP_NOMEMALLOC) + strlcat (THIS->__retvalue, "|GFP_NOMEMALLOC",MAXSTRINGLEN); +#endif + +#ifdef __GFP_HARDWALL + if (flags & __GFP_HARDWALL) + strlcat (THIS->__retvalue, "|GFP_HARDWALL",MAXSTRINGLEN); +#endif + +#ifdef __GFP_THISNODE + if (flags & __GFP_THISNODE) + strlcat (THIS->__retvalue, "|GFP_THISNODE",MAXSTRINGLEN); +#endif + +#ifdef __GFP_RECLAIMABLE + if (flags & __GFP_RECLAIMABLE) + strlcat (THIS->__retvalue, "|GFP_RECLAIMABLE",MAXSTRINGLEN); +#endif + + +#ifdef __GFP_NOTRACK + if (flags & __GFP_NOTRACK) + strlcat (THIS->__retvalue, "|GFP_NOTRACK",MAXSTRINGLEN); +#endif +%} + + +/** + * probe kmem.kmalloc - Fires when <command>kmalloc</command> is requested. + * @call_site: Address of the kmemory function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: type of kmemory to allocate + * @ptr: Pointer to the kmemory allocated + */ + +probe kmem.kmalloc = kernel.trace("kmalloc") { + name = "kmalloc" + call_site = symname($call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_f($gfp_flags) + ptr = $ptr +} + +/** + * probe kmem.kmem_cache_alloc - Fires when \ + * <command>kmem_cache_alloc</command> is requested. + * @call_site: Address of the function calling this kmemory function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of kmemory to allocate + * @ptr: Pointer to the kmemory allocated + */ +probe kmem.kmem_cache_alloc = kernel.trace("kmem_cache_alloc") { + name = "kmem_cache_alloc" + call_site = symname($call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_f($gfp_flags) + ptr = $ptr +} + + +/** + * probe kmem.kmalloc_node - Fires when <command>kmalloc_node</command> is requested. + * @call_site: Address of the function caling this kmemory function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of kmemory to allocate + * @ptr: Pointer to the kmemory allocated + */ +probe kmem.kmalloc_node = kernel.trace("kmalloc_node")? { + name = "kmalloc_node" + call_site = symname($call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_f($gfp_flags) + ptr = $ptr +} + +/** + * probe kmem.kmem_cache_alloc_node - Fires when \ + * <command>kmem_cache_alloc_node</command> is requested. + * @call_site: Address of the function calling this kmemory function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of kmemory to allocate + * @ptr: Pointer to the kmemory allocated + */ +probe kmem.kmem_cache_alloc_node = kernel.trace("kmem_cache_alloc_node")? { + name = "kmem_cache_alloc_node" + call_site = symname($call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags =gfp_f($gfp_flags) + ptr = $ptr +} + +/** + * probe kmem.kfree - Fires when <command>kfree</comand> is requested. + * @call_site: Address of the function calling this kmemory function. + * @ptr: Pointer to the kmemory allocated which is returned by kmalloc + */ +probe kmem.kfree.kp = kernel.function("kfree") { + name = "kfree" + call_site = symname(call_site()) + ptr = $x +} + +probe kmem.kfree.tp = kernel.trace("kfree") { + name = "kfree" + call_site = symname($call_site) + ptr = $ptr +} + +probe kmem.kfree = kmem.kfree.tp !, + kmem.kfree.kp +{} + +/** + * probe kmem.kmem_cache_free - Fires when \ + * <command>kmem_cache_free</command> is requested. + * @call_site: Address of the function calling this kmemory function. + * @ptr: Pointer to the kmemory allocated which is returned by kmem_cache + */ +probe kmem.kmem_cache_free = kernel.trace("kmem_cache_free") { + name = "kmem_cache_free" + call_site = symname($call_site) + ptr = $ptr +} diff -rupaN a/testsuite/buildok/kmem_tracepoints.stp b/testsuite/buildok/kmem_tracepoints.stp --- a/testsuite/buildok/kmem_tracepoints.stp 1970-01-01 01:00:00.000000000 +0100 +++ b/testsuite/buildok/kmem_tracepoints.stp 2009-09-24 19:13:40.000000000 +0200 @@ -0,0 +1,34 @@ +#!/usr/bin/stp -up4 + +probe kmem.kfree { + println (name) + printf("%-15s %-15s %-15p \n", execname(),call_site,ptr) +} + +probe kmem.kmalloc { + println (name) + printf("%-15s %-15s %-15p %-15d %-15d %-15s \n", execname(),call_site,ptr,bytes_req,bytes_alloc,gfp_flags) +} + +probe kmem.kmem_cache_alloc { + println (name) + printf("%-15s %-15s %-15p %-15d %-15d %-15s \n", execname(),call_site,ptr,bytes_req,bytes_alloc,gfp_flags) +} +probe kmem.kmalloc_node { + println (name) + printf("%-15s %-15s %-15p %-15d %-15d %-15s \n", execname(),call_site,ptr,bytes_req,bytes_alloc,gfp_flags) +} + +probe kmem.kmem_cache_alloc_node { + println (name) + printf("%-15s %-15s %-15p %-15d %-15d %-15s \n", execname(),call_site,ptr,bytes_req,bytes_alloc,gfp_flags) +} + +probe kmem.kmem_cache_free { + println (name) + printf("%-15s %-15s %-15p \n", execname(),call_site,ptr) +} + +probe timer.s(1) { + exit() +} -- Rajasekhar Duddu (rajduddu@linux.vnet.ibm.com), Linux on System z - CSVT, IBM LTC, Bangalore. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Tracepoint Tapset for Memory Subsystem 2009-09-24 18:08 ` [PATCH v2] " Rajasekhar Duddu @ 2009-09-25 19:19 ` David Smith 2009-09-25 20:07 ` Frank Ch. Eigler 2009-09-28 18:12 ` Jim Keniston 2009-09-25 21:50 ` Josh Stone 1 sibling, 2 replies; 18+ messages in thread From: David Smith @ 2009-09-25 19:19 UTC (permalink / raw) To: Rajasekhar Duddu; +Cc: systemtap, Jim Keniston On 09/24/2009 01:08 PM, Rajasekhar Duddu wrote: > > Hi, I have modified the patch according to the comments passed > by Frank and David. > > Many thanks to Frank And David. See comments below. > Signed-off-by: Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> > diff -rupaN a/tapset/memory.stp b/tapset/memory.stp > --- a/tapset/memory.stp 2009-09-24 19:01:10.000000000 +0200 > +++ b/tapset/memory.stp 2009-09-24 19:13:30.000000000 +0200 > @@ -195,3 +195,205 @@ probe vm.brk = kernel.function("do_brk") > probe vm.oom_kill = kernel.function("__oom_kill_task") { > task = $p > } > +#Function to get the call_site for kprobe based kfree probe. > + > +function call_site:long () > +%{ > + int *ptr ; > + ptr = __builtin_return_address(0); > + THIS->__retvalue = *ptr; > +%} Sorry to keep finding more things, but... I'm a bit worried about your use of '__builtin_return_address()' here. Jim Keniston reported on it back in 2005 in the following message, but there isn't much context. <http://sourceware.org/ml/systemtap/2005-q2/msg00242.html> Jim, can you remember some context here? Was the use of '__builtin_return_address' considered good/bad/neutral? We don't seem to use it anywhere else. > + > + > +/*Function to convert the GFP_FLAGS .*/ > +function gfp_f:string (gfp_flag:long) > +%{ > +int flags = (int)THIS->gfp_flag; > + > + > +#ifdef __GFP_HIGH > + if (flags & __GFP_HIGH) > + strlcat (THIS->__retvalue, "GFP_HIGH",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_WAIT > + if (flags & __GFP_WAIT) > + strlcat (THIS->__retvalue, "GFP_WAIT",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_IO > + if (flags & __GFP_IO) > + strlcat (THIS->__retvalue, "|GFP_IO",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_FS > + if (flags & __GFP_FS) > + strlcat (THIS->__retvalue, "|GFP_FS",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_COLD > + if (flags & __GFP_COLD) > + strlcat (THIS->__retvalue, "|GFP_COLD",MAXSTRINGLEN); > +#endif > +#ifdef __GFP_NOWARN > + if (flags & __GFP_NOWARN) > + strlcat (THIS->__retvalue, "|GFP_NOWARN",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_REPEAT > + if (flags & __GFP_REPEAT) > + strlcat (THIS->__retvalue, "|GFP_REPEAT",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_NOFAIL > + if (flags & __GFP_NOFAIL) > + strlcat (THIS->__retvalue, "|GFP_NOFAIL",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_NORETRY > + if (flags & __GFP_NORETRY) > + strlcat (THIS->__retvalue, "|GFP_NORETRY",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_COMP > + if (flags & __GFP_COMP) > + strlcat (THIS->__retvalue, "|GFP_COMP",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_ZERO > + if (flags & __GFP_ZERO) > + strlcat (THIS->__retvalue, "|GFP_ZERO",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_NOMEMALLOC > + if (flags & __GFP_NOMEMALLOC) > + strlcat (THIS->__retvalue, "|GFP_NOMEMALLOC",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_HARDWALL > + if (flags & __GFP_HARDWALL) > + strlcat (THIS->__retvalue, "|GFP_HARDWALL",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_THISNODE > + if (flags & __GFP_THISNODE) > + strlcat (THIS->__retvalue, "|GFP_THISNODE",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_RECLAIMABLE > + if (flags & __GFP_RECLAIMABLE) > + strlcat (THIS->__retvalue, "|GFP_RECLAIMABLE",MAXSTRINGLEN); > +#endif > + > + > +#ifdef __GFP_NOTRACK > + if (flags & __GFP_NOTRACK) > + strlcat (THIS->__retvalue, "|GFP_NOTRACK",MAXSTRINGLEN); > +#endif > +%} The above function looks good. However, since you are using strlcat exclusively, I'd probably start with this: THIS->__retvalue[0] = '\0'; I'm probably being paranoid here, but it can't hurt. Here's another minor point. I'd probably call this function something like '_gfp_flag_str' (the '_f' suffix doesn't mean much to me). ... stuff deleted ... > +/** > + * probe kmem.kfree - Fires when <command>kfree</comand> is requested. > + * @call_site: Address of the function calling this kmemory function. > + * @ptr: Pointer to the kmemory allocated which is returned by kmalloc > + */ > +probe kmem.kfree.kp = kernel.function("kfree") { > + name = "kfree" > + call_site = symname(call_site()) > + ptr = $x > +} > + > +probe kmem.kfree.tp = kernel.trace("kfree") { > + name = "kfree" > + call_site = symname($call_site) > + ptr = $ptr > +} > + > +probe kmem.kfree = kmem.kfree.tp !, > + kmem.kfree.kp > +{} At first I didn't see the need for the 2 separate probes, but I got it. -- David Smith dsmith@redhat.com Red Hat http://www.redhat.com 256.217.0141 (direct) 256.837.0057 (fax) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Tracepoint Tapset for Memory Subsystem 2009-09-25 19:19 ` David Smith @ 2009-09-25 20:07 ` Frank Ch. Eigler 2009-09-28 18:12 ` Jim Keniston 1 sibling, 0 replies; 18+ messages in thread From: Frank Ch. Eigler @ 2009-09-25 20:07 UTC (permalink / raw) To: David Smith; +Cc: Rajasekhar Duddu, systemtap, Jim Keniston > [...] >> +#Function to get the call_site for kprobe based kfree probe. >> + >> +function call_site:long () >> +%{ >> + int *ptr ; >> + ptr = __builtin_return_address(0); >> + THIS->__retvalue = *ptr; >> +%} > [...] This is not reliable and should be avoided. When PR6961 & PR6580 are figured out, this should be directly available as caller_addr() or perhaps stack(2). - FChE ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Tracepoint Tapset for Memory Subsystem 2009-09-25 19:19 ` David Smith 2009-09-25 20:07 ` Frank Ch. Eigler @ 2009-09-28 18:12 ` Jim Keniston 2009-09-29 8:58 ` K.Prasad 1 sibling, 1 reply; 18+ messages in thread From: Jim Keniston @ 2009-09-28 18:12 UTC (permalink / raw) To: David Smith; +Cc: Rajasekhar Duddu, systemtap On Fri, 2009-09-25 at 14:19 -0500, David Smith wrote: ... > > Sorry to keep finding more things, but... > > I'm a bit worried about your use of '__builtin_return_address()' here. > Jim Keniston reported on it back in 2005 in the following message, but > there isn't much context. > > <http://sourceware.org/ml/systemtap/2005-q2/msg00242.html> > > Jim, can you remember some context here? Was the use of > '__builtin_return_address' considered good/bad/neutral? We don't seem > to use it anywhere else. > In case anybody still cares... The context was that we had recently implemented kretprobes, and somebody pointed out that hijacking the return address would cause __builtin_return_address() to return the wrong value. From my survey of the kernel, I concluded that "__builtin_return_address is used entirely for tracing (tracing that is disabled by default), profiling, and error reporting. I couldn't find any case where normal operation of the OS would be affected." Ironically, soon after that, kprobes itself started using __builtin_return_address(). Anyway, there was no controversy as to whether __builtin_return_address() was bad or good per se; it was simply recognized that it would return invalid results when called from a return-probed function. Jim ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Tracepoint Tapset for Memory Subsystem 2009-09-28 18:12 ` Jim Keniston @ 2009-09-29 8:58 ` K.Prasad 0 siblings, 0 replies; 18+ messages in thread From: K.Prasad @ 2009-09-29 8:58 UTC (permalink / raw) Cc: David Smith, Rajasekhar Duddu, systemtap On Mon, Sep 28, 2009 at 11:12:28AM -0700, Jim Keniston wrote: > > On Fri, 2009-09-25 at 14:19 -0500, David Smith wrote: > ... > > > > Sorry to keep finding more things, but... > > > > I'm a bit worried about your use of '__builtin_return_address()' here. > > Jim Keniston reported on it back in 2005 in the following message, but > > there isn't much context. > > > > <http://sourceware.org/ml/systemtap/2005-q2/msg00242.html> > > > > Jim, can you remember some context here? Was the use of > > '__builtin_return_address' considered good/bad/neutral? We don't seem > > to use it anywhere else. > > > > In case anybody still cares... > Yes, your explanation actually helped! > The context was that we had recently implemented kretprobes, and > somebody pointed out that hijacking the return address would cause > __builtin_return_address() to return the wrong value. From my survey of > the kernel, I concluded that "__builtin_return_address is used entirely > for tracing (tracing that is disabled by default), profiling, and error > reporting. I couldn't find any case where normal operation of the OS > would be affected." > > Ironically, soon after that, kprobes itself started using > __builtin_return_address(). > > Anyway, there was no controversy as to whether > __builtin_return_address() was bad or good per se; it was simply > recognized that it would return invalid results when called from a > return-probed function. > > Jim > This means that __builtin_return_address() would return incorrect values irrespective of whether it is used inside a kprobe or a tracepoint based probe i.e. "kmem.kfree.kp" or "kmem.kfree.tp". And since the tracepoints export them (through $call_site parameter), I think we can continue to use them in the kprobe based fallback probe too. Thanks, K.Prasad ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Tracepoint Tapset for Memory Subsystem 2009-09-24 18:08 ` [PATCH v2] " Rajasekhar Duddu 2009-09-25 19:19 ` David Smith @ 2009-09-25 21:50 ` Josh Stone 2009-09-30 10:12 ` Rajasekhar Duddu 1 sibling, 1 reply; 18+ messages in thread From: Josh Stone @ 2009-09-25 21:50 UTC (permalink / raw) To: Rajasekhar Duddu; +Cc: systemtap On 09/24/2009 11:08 AM, Rajasekhar Duddu wrote: > > Hi, I have modified the patch according to the comments passed > by Frank and David. > > Many thanks to Frank And David. > > Changelog: > Removed the Hardcoded constants in converting GFPFLAGS. > Added a kprobe based fallback probe to kfree. Why is kfree the only one you're adding with a kprobe fallback? This is a good feature to have! > +/** > + * probe kmem.kfree - Fires when <command>kfree</comand> is requested. > + * @call_site: Address of the function calling this kmemory function. > + * @ptr: Pointer to the kmemory allocated which is returned by kmalloc > + */ > +probe kmem.kfree.kp = kernel.function("kfree") { > + name = "kfree" > + call_site = symname(call_site()) > + ptr = $x > +} > + > +probe kmem.kfree.tp = kernel.trace("kfree") { > + name = "kfree" > + call_site = symname($call_site) > + ptr = $ptr > +} > + > +probe kmem.kfree = kmem.kfree.tp !, > + kmem.kfree.kp > +{} Please use an underscored prefix (e.g. __kmem) for internal details that the user isn't meant to use directly, like the .tp/.kp branches. I'd also like to see a consistent prefix in this tapset. The existing probe points in memory.stp are all vm.*, so it's probably best to stick with that. Thanks, Josh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] Tracepoint Tapset for Memory Subsystem 2009-09-25 21:50 ` Josh Stone @ 2009-09-30 10:12 ` Rajasekhar Duddu 2009-10-02 15:14 ` [PATCH v3] " Rajasekhar Duddu 0 siblings, 1 reply; 18+ messages in thread From: Rajasekhar Duddu @ 2009-09-30 10:12 UTC (permalink / raw) To: Josh Stone; +Cc: systemtap On Fri, Sep 25, 2009 at 02:50:35PM -0700, Josh Stone wrote: > > Changelog: > > Removed the Hardcoded constants in converting GFPFLAGS. > > Added a kprobe based fallback probe to kfree. > > Why is kfree the only one you're adding with a kprobe fallback? > This is > a good feature to have! > Hi Josh, many thanks for your comments, I have tried to have fallback probes for all the tracepoints, but I found that the variables exported by the other tracepoints(except kfree) are getting changed, so I could not have fallback for all of them. I will modify my patch according to your other comments. Thanks -- Rajasekhar Duddu (rajduddu@linux.vnet.ibm.com), Linux on System z - CSVT, IBM LTC, Bangalore. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] Tracepoint Tapset for Memory Subsystem 2009-09-30 10:12 ` Rajasekhar Duddu @ 2009-10-02 15:14 ` Rajasekhar Duddu 2009-10-06 19:01 ` Frank Ch. Eigler 0 siblings, 1 reply; 18+ messages in thread From: Rajasekhar Duddu @ 2009-10-02 15:14 UTC (permalink / raw) To: systemtap Hi, Thanks for all those who had sent their comments. I have modified the patch according to the comments passed. Signed-off-by: Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> diff -rupaN a/tapset/memory.stp b/tapset/memory.stp --- a/tapset/memory.stp 2009-09-30 06:14:29.000000000 -0400 +++ b/tapset/memory.stp 2009-10-02 11:05:10.000000000 -0400 @@ -195,3 +195,209 @@ probe vm.brk = kernel.function("do_brk") probe vm.oom_kill = kernel.function("__oom_kill_task") { task = $p } +/* Function to convert the GFP_FLAGS . */ + +function gfp_flag_str:string (gfp_flag:long) +%{ +int flags = (int)THIS->gfp_flag; +THIS->__retvalue[0] = '\0'; + +#ifdef __GFP_HIGH + if (flags & __GFP_HIGH) + strlcat (THIS->__retvalue, "GFP_HIGH",MAXSTRINGLEN); +#endif + +#ifdef __GFP_WAIT + if (flags & __GFP_WAIT) + strlcat (THIS->__retvalue, "GFP_WAIT",MAXSTRINGLEN); +#endif + +#ifdef __GFP_IO + if (flags & __GFP_IO) + strlcat (THIS->__retvalue, "|GFP_IO",MAXSTRINGLEN); +#endif + +#ifdef __GFP_FS + if (flags & __GFP_FS) + strlcat (THIS->__retvalue, "|GFP_FS",MAXSTRINGLEN); +#endif + +#ifdef __GFP_COLD + if (flags & __GFP_COLD) + strlcat (THIS->__retvalue, "|GFP_COLD",MAXSTRINGLEN); +#endif +#ifdef __GFP_NOWARN + if (flags & __GFP_NOWARN) + strlcat (THIS->__retvalue, "|GFP_NOWARN",MAXSTRINGLEN); +#endif + +#ifdef __GFP_REPEAT + if (flags & __GFP_REPEAT) + strlcat (THIS->__retvalue, "|GFP_REPEAT",MAXSTRINGLEN); +#endif + +#ifdef __GFP_NOFAIL + if (flags & __GFP_NOFAIL) + strlcat (THIS->__retvalue, "|GFP_NOFAIL",MAXSTRINGLEN); +#endif + +#ifdef __GFP_NORETRY + if (flags & __GFP_NORETRY) + strlcat (THIS->__retvalue, "|GFP_NORETRY",MAXSTRINGLEN); +#endif + +#ifdef __GFP_COMP + if (flags & __GFP_COMP) + strlcat (THIS->__retvalue, "|GFP_COMP",MAXSTRINGLEN); +#endif + +#ifdef __GFP_ZERO + if (flags & __GFP_ZERO) + strlcat (THIS->__retvalue, "|GFP_ZERO",MAXSTRINGLEN); +#endif + +#ifdef __GFP_NOMEMALLOC + if (flags & __GFP_NOMEMALLOC) + strlcat (THIS->__retvalue, "|GFP_NOMEMALLOC",MAXSTRINGLEN); +#endif + +#ifdef __GFP_HARDWALL + if (flags & __GFP_HARDWALL) + strlcat (THIS->__retvalue, "|GFP_HARDWALL",MAXSTRINGLEN); +#endif + +#ifdef __GFP_THISNODE + if (flags & __GFP_THISNODE) + strlcat (THIS->__retvalue, "|GFP_THISNODE",MAXSTRINGLEN); +#endif + +#ifdef __GFP_RECLAIMABLE + if (flags & __GFP_RECLAIMABLE) + strlcat (THIS->__retvalue, "|GFP_RECLAIMABLE",MAXSTRINGLEN); +#endif + + +#ifdef __GFP_NOTRACK + if (flags & __GFP_NOTRACK) + strlcat (THIS->__retvalue, "|GFP_NOTRACK",MAXSTRINGLEN); +#endif +%} + + +/** + * probe vm.kmalloc - Fires when <command>kmalloc</command> is requested. + * @call_site: Address of the caller function. + * @caller_function: Name of the caller function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: type of kmemory to allocate + * @ptr: Pointer to the kmemory allocated + */ + +probe vm.kmalloc = kernel.trace("kmalloc") { + name = "kmalloc" + call_site = $call_site + caller_function = symname(call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_flag_str($gfp_flags) + ptr = $ptr +} + +/** + * probe vm.kmem_cache_alloc - Fires when \ + * <command>kmem_cache_alloc</command> is requested. + * @call_site: Address of the caller function. + * @caller_function: Name of the caller function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of kmemory to allocate + * @ptr: Pointer to the kmemory allocated + */ +probe vm.kmem_cache_alloc = kernel.trace("kmem_cache_alloc") { + name = "kmem_cache_alloc" + call_site = $call_site + caller_function = symname(call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_flag_str($gfp_flags) + ptr = $ptr +} + + +/** + * probe vm.kmalloc_node - Fires when <command>kmalloc_node</command> is requested. + * @call_site: Address of the caller function. + * @caller_function: Name of the caller function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of kmemory to allocate + * @ptr: Pointer to the kmemory allocated + */ +probe vm.kmalloc_node = kernel.trace("kmalloc_node")? { + name = "kmalloc_node" + call_site = $call_site + caller_function = symname(call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags = gfp_flag_str($gfp_flags) + ptr = $ptr +} + +/** + * probe vm.kmem_cache_alloc_node - Fires when \ + * <command>kmem_cache_alloc_node</command> is requested. + * @call_site: Address of the caller function. + * @caller_function: Name of the caller function. + * @bytes_req: Requested Bytes + * @bytes_alloc: Allocated Bytes + * @gfp_flags: Type of kmemory to allocate + * @ptr: Pointer to the kmemory allocated + */ +probe vm.kmem_cache_alloc_node = kernel.trace("kmem_cache_alloc_node")? { + name = "kmem_cache_alloc_node" + call_site = $call_site + caller_function = symname(call_site) + bytes_req = $bytes_req + bytes_alloc = $bytes_alloc + gfp_flags =gfp_flag_str($gfp_flags) + ptr = $ptr +} + +probe __vm.kfree.kp = kernel.function("kfree") { + name = "kfree" + call_site = "0" + caller_function = "unknown" + ptr = $x +} + +probe __vm.kfree.tp = kernel.trace("kfree") { + name = "kfree" + call_site = $call_site + caller_function = symname(call_site) + ptr = $ptr +} + +/** + * probe vm.kfree - Fires when <command>kfree</comand> is requested. + * @call_site: Address of the caller function (displayed if available) + * @caller_function - Name of the caller function (displayed if available) + * @ptr: Pointer to the kmemory allocated which is returned by kmalloc + */ +probe vm.kfree = __vm.kfree.tp !, + __vm.kfree.kp +{} + +/** + * probe vm.kmem_cache_free - Fires when \ + * <command>kmem_cache_free</command> is requested. + * @call_site: Address of the caller function. + * @caller_function: Name of the caller function. + * @ptr: Pointer to the kmemory allocated which is returned by kmem_cache + */ +probe vm.kmem_cache_free = kernel.trace("kmem_cache_free") { + name = "kmem_cache_free" + call_site = $call_site + caller_function = symname(call_site) + ptr = $ptr +} diff -rupaN a/testsuite/buildok/vm.tracepoints.stp b/testsuite/buildok/vm.tracepoints.stp --- a/testsuite/buildok/vm.tracepoints.stp 1969-12-31 19:00:00.000000000 -0500 +++ b/testsuite/buildok/vm.tracepoints.stp 2009-10-02 10:59:20.000000000 -0400 @@ -0,0 +1,32 @@ +#!/usr/bin/stp -up4 + +probe vm.kfree { + println (name) + printf("%-15s %-15p %-15s %-15p \n", execname(), call_site, caller_function, ptr) +} + +probe vm.kmalloc { + println (name) + printf("%-15s %-15p %-15s %-15p %-15d %-15d %-15s \n", execname(), call_site, caller_function, ptr, bytes_req, bytes_alloc, gfp_flags) +} + +probe vm.kmem_cache_alloc { + println (name) + printf("%-15s %-15p %-15s %-15p %-15d %-15d %-15s \n", execname(), call_site, caller_function, ptr, bytes_req, bytes_alloc, gfp_flags) +} +probe vm.kmalloc_node { + println (name) + printf("%-15s %-15p %-15s %-15p %-15d %-15d %-15s \n", execname(), call_site, caller_function, ptr, bytes_req, bytes_alloc, gfp_flags) +} + +probe vm.kmem_cache_alloc_node { + println (name) + printf("%-15s %-15p %-15s %-15p %-15d %-15d %-15s \n", execname(), call_site, caller_function, ptr, bytes_req, bytes_alloc, gfp_flags) +} + +probe vm.kmem_cache_free { + println (name) + printf("%-15s %-15p %-15s %-15p \n", execname(), call_site, caller_function, ptr) +} + + Thanks -- Rajasekhar Duddu (rajduddu@linux.vnet.ibm.com), Linux on System z - CSVT, IBM LTC, Bangalore. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] Tracepoint Tapset for Memory Subsystem 2009-10-02 15:14 ` [PATCH v3] " Rajasekhar Duddu @ 2009-10-06 19:01 ` Frank Ch. Eigler 2009-10-07 13:07 ` Rajasekhar Duddu 0 siblings, 1 reply; 18+ messages in thread From: Frank Ch. Eigler @ 2009-10-06 19:01 UTC (permalink / raw) To: Rajasekhar Duddu; +Cc: systemtap Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> writes: > [...] > +/* Function to convert the GFP_FLAGS . */ > + > +function gfp_flag_str:string (gfp_flag:long) > +%{ > +int flags = (int)THIS->gfp_flag; > +THIS->__retvalue[0] = '\0'; > + > +#ifdef __GFP_HIGH > + if (flags & __GFP_HIGH) > + strlcat (THIS->__retvalue, "GFP_HIGH",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_WAIT > + if (flags & __GFP_WAIT) > + strlcat (THIS->__retvalue, "GFP_WAIT",MAXSTRINGLEN); > +#endif > + > +#ifdef __GFP_IO > + if (flags & __GFP_IO) > + strlcat (THIS->__retvalue, "|GFP_IO",MAXSTRINGLEN); > +#endif Why no "|" before GFP_HIGH/GFP_WAIT? Also, why no "__" before the stringified version? > +#ifdef __GFP_FS > + if (flags & __GFP_FS) > + strlcat (THIS->__retvalue, "|GFP_FS",MAXSTRINGLEN); > +#endif (How about a macro to generate all these near-identical branches?) > +%} > +/** > + * probe vm.kmalloc - Fires when <command>kmalloc</command> is requested. > + * @call_site: Address of the caller function. > + * @caller_function: Name of the caller function. > + * @bytes_req: Requested Bytes > + * @bytes_alloc: Allocated Bytes > + * @gfp_flags: type of kmemory to allocate > + * @ptr: Pointer to the kmemory allocated > + */ > + > +probe vm.kmalloc = kernel.trace("kmalloc") { > + name = "kmalloc" > + call_site = $call_site > + caller_function = symname(call_site) > + bytes_req = $bytes_req > + bytes_alloc = $bytes_alloc > + gfp_flags = gfp_flag_str($gfp_flags) > + ptr = $ptr > +} Nice. I thought that the raison d'etre for these aliases was to abstract the presence or absence of tracepoints, so is there no fallback kprobe available? Something like this: > +probe __vm.kfree.kp = kernel.function("kfree") { > + name = "kfree" > + call_site = "0" (Note though that this will fail type checking on a non-tracepoint kernel -- have you tried it? -- it should be just 0 instead of "0".) > + caller_function = "unknown" > + ptr = $x > +} > + > +probe __vm.kfree.tp = kernel.trace("kfree") { > + name = "kfree" > + call_site = $call_site > + caller_function = symname(call_site) > + ptr = $ptr > +} > + > +/** > + * probe vm.kfree - Fires when <command>kfree</comand> is requested. > + * @call_site: Address of the caller function (displayed if available) > + * @caller_function - Name of the caller function (displayed if available) > + * @ptr: Pointer to the kmemory allocated which is returned by kmalloc > + */ > +probe vm.kfree = __vm.kfree.tp !, > + __vm.kfree.kp > +{} Right. > +/** > + * probe vm.kmalloc_node - Fires when <command>kmalloc_node</command> is requested. > + * @call_site: Address of the caller function. > + * @caller_function: Name of the caller function. > + * @bytes_req: Requested Bytes > + * @bytes_alloc: Allocated Bytes > + * @gfp_flags: Type of kmemory to allocate > + * @ptr: Pointer to the kmemory allocated > + */ Please, no "<command>" markup in there, it is not valid. > +probe vm.kmalloc_node = kernel.trace("kmalloc_node")? { > [...] Why is this marked with "?"? > --- a/testsuite/buildok/vm.tracepoints.stp 1969-12-31 19:00:00.000000000 -0500 > +++ b/testsuite/buildok/vm.tracepoints.stp 2009-10-02 10:59:20.000000000 -0400 > @@ -0,0 +1,32 @@ > +#!/usr/bin/stp -up4 Other similar test cases just use #! stap -up4 - FChE ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] Tracepoint Tapset for Memory Subsystem 2009-10-06 19:01 ` Frank Ch. Eigler @ 2009-10-07 13:07 ` Rajasekhar Duddu 2009-10-07 19:51 ` Frank Ch. Eigler 0 siblings, 1 reply; 18+ messages in thread From: Rajasekhar Duddu @ 2009-10-07 13:07 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: systemtap Hi Frank, thanks for your comments. I am answering for your questions here and in my next patch I will apply all the possible changes. On Tue, Oct 06, 2009 at 03:01:09PM -0400, Frank Ch. Eigler wrote: > Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> writes: > > > [...] > > +/* Function to convert the GFP_FLAGS . */ > > + > > +function gfp_flag_str:string (gfp_flag:long) > > +%{ > > +int flags = (int)THIS->gfp_flag; > > +THIS->__retvalue[0] = '\0'; > > + > > +#ifdef __GFP_HIGH > > + if (flags & __GFP_HIGH) > > + strlcat (THIS->__retvalue, "GFP_HIGH",MAXSTRINGLEN); > > +#endif > > + > > +#ifdef __GFP_WAIT > > + if (flags & __GFP_WAIT) > > + strlcat (THIS->__retvalue, "GFP_WAIT",MAXSTRINGLEN); > > +#endif > > + > > +#ifdef __GFP_IO > > + if (flags & __GFP_IO) > > + strlcat (THIS->__retvalue, "|GFP_IO",MAXSTRINGLEN); > > +#endif > > Why no "|" before GFP_HIGH/GFP_WAIT? All the other Flags (except GFP_WAIT/GFP_HIGH) are composition of one or the other Flags, and the composition starts with either GFP_HIGH or GFP_WAIT, so there is no "|" before GFP_HIGH/GFP_WAIT. > Also, why no "__" before the stringified version? This I will apply in the next patch. > > > > +#ifdef __GFP_FS > > + if (flags & __GFP_FS) > > + strlcat (THIS->__retvalue, "|GFP_FS",MAXSTRINGLEN); > > +#endif > > (How about a macro to generate all these near-identical branches?) > > Sure I will have macro something like this: %{ #define FLAG_TEST(TYPE) if (flags & TYPE) { strlcat \ (THIS->__retvalue, #TYPE,MAXSTRINGLEN); } %} and I will use that in my gfp_flag_str as fallowing. #ifdef __GFP_HIGH FLAG_TEST(__GFP_HIGH); #endif > > +%} > > > > +/** > > + * probe vm.kmalloc - Fires when <command>kmalloc</command> is requested. > > + * @call_site: Address of the caller function. > > + * @caller_function: Name of the caller function. > > + * @bytes_req: Requested Bytes > > + * @bytes_alloc: Allocated Bytes > > + * @gfp_flags: type of kmemory to allocate > > + * @ptr: Pointer to the kmemory allocated > > + */ > > + > > +probe vm.kmalloc = kernel.trace("kmalloc") { > > + name = "kmalloc" > > + call_site = $call_site > > + caller_function = symname(call_site) > > + bytes_req = $bytes_req > > + bytes_alloc = $bytes_alloc > > + gfp_flags = gfp_flag_str($gfp_flags) > > + ptr = $ptr > > +} > > Nice. I thought that the raison d'etre for these aliases was to > abstract the presence or absence of tracepoints, so is there no > fallback kprobe available? Something like this: > Fallback kprobe is not available for other memory functions because the variables exported by them are will be modified. > > > +probe __vm.kfree.kp = kernel.function("kfree") { > > + name = "kfree" > > + call_site = "0" > > (Note though that this will fail type checking on a non-tracepoint > kernel -- have you tried it? -- it should be just 0 instead of "0".) > This also I will apply in my next patch. > > + caller_function = "unknown" > > + ptr = $x > > +} > > + > > +probe __vm.kfree.tp = kernel.trace("kfree") { > > + name = "kfree" > > + call_site = $call_site > > + caller_function = symname(call_site) > > + ptr = $ptr > > +} > > + > > +/** > > + * probe vm.kfree - Fires when <command>kfree</comand> is requested. > > + * @call_site: Address of the caller function (displayed if available) > > + * @caller_function - Name of the caller function (displayed if available) > > + * @ptr: Pointer to the kmemory allocated which is returned by kmalloc > > + */ > > +probe vm.kfree = __vm.kfree.tp !, > > + __vm.kfree.kp > > +{} > > Right. > > > > +/** > > + * probe vm.kmalloc_node - Fires when <command>kmalloc_node</command> is requested. > > + * @call_site: Address of the caller function. > > + * @caller_function: Name of the caller function. > > + * @bytes_req: Requested Bytes > > + * @bytes_alloc: Allocated Bytes > > + * @gfp_flags: Type of kmemory to allocate > > + * @ptr: Pointer to the kmemory allocated > > + */ > > Please, no "<command>" markup in there, it is not valid. > This I will cange in my next patch. > > > +probe vm.kmalloc_node = kernel.trace("kmalloc_node")? { > > [...] > > Why is this marked with "?"? Kmalloc_node will be called when "CONFIG_NUMA" is defined, so I have used "?" to make it an optional. > > > > --- a/testsuite/buildok/vm.tracepoints.stp 1969-12-31 19:00:00.000000000 -0500 > > +++ b/testsuite/buildok/vm.tracepoints.stp 2009-10-02 10:59:20.000000000 -0400 > > @@ -0,0 +1,32 @@ > > +#!/usr/bin/stp -up4 > > Other similar test cases just use > > #! stap -up4 > This also I will change in my next patch. > Thanks -- Rajasekhar Duddu (rajduddu@linux.vnet.ibm.com), Linux on System z - CSVT, IBM LTC, Bangalore. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] Tracepoint Tapset for Memory Subsystem 2009-10-07 13:07 ` Rajasekhar Duddu @ 2009-10-07 19:51 ` Frank Ch. Eigler 2009-10-09 17:08 ` Rajasekhar Duddu 0 siblings, 1 reply; 18+ messages in thread From: Frank Ch. Eigler @ 2009-10-07 19:51 UTC (permalink / raw) To: Rajasekhar Duddu; +Cc: systemtap Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> writes: > [...] >> Nice. I thought that the raison d'etre for these aliases was to >> abstract the presence or absence of tracepoints, so is there no >> fallback kprobe available? Something like this: >> > Fallback kprobe is not available for other memory functions because > the variables exported by them are will be modified. Could you elaborate? Do you mean that the same values may not be available from a kprobe context? - FChE ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] Tracepoint Tapset for Memory Subsystem 2009-10-07 19:51 ` Frank Ch. Eigler @ 2009-10-09 17:08 ` Rajasekhar Duddu 2009-10-09 17:38 ` Frank Ch. Eigler 0 siblings, 1 reply; 18+ messages in thread From: Rajasekhar Duddu @ 2009-10-09 17:08 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: systemtap On Wed, Oct 07, 2009 at 03:51:07PM -0400, Frank Ch. Eigler wrote: > Rajasekhar Duddu <rajduddu@linux.vnet.ibm.com> writes: > > > [...] > >> Nice. I thought that the raison d'etre for these aliases was to > >> abstract the presence or absence of tracepoints, so is there no > >> fallback kprobe available? Something like this: > >> > > Fallback kprobe is not available for other memory functions because > > the variables exported by them are will be modified. > > Could you elaborate? Do you mean that the same values may not be > available from a kprobe context? > > Hi Frank, Yes, the same values may not be available from a kprobe context, for example if we take "ret" variable as it is populated mid-way in the function and it is also the return value of a function which can be captured only by a return probe. But by a return probe we cannot capture the formal parameters of the memory function. Thanks -- Rajasekhar Duddu (rajduddu@linux.vnet.ibm.com), Linux on System z - CSVT, IBM LTC, Bangalore. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] Tracepoint Tapset for Memory Subsystem 2009-10-09 17:08 ` Rajasekhar Duddu @ 2009-10-09 17:38 ` Frank Ch. Eigler 2009-10-14 8:32 ` Rajasekhar Duddu 0 siblings, 1 reply; 18+ messages in thread From: Frank Ch. Eigler @ 2009-10-09 17:38 UTC (permalink / raw) To: Rajasekhar Duddu; +Cc: systemtap Hi - On Fri, Oct 09, 2009 at 10:38:05PM +0530, Rajasekhar Duddu wrote: > [...] > > > Fallback kprobe is not available for other memory functions because > > > the variables exported by them are will be modified. > > > > Could you elaborate? Do you mean that the same values may not be > > available from a kprobe context? > Yes, the same values may not be available from a kprobe > context, for example if we take "ret" variable as it is populated mid-way in > the function and it is also the return value of a function which can > be captured only by a return probe. But by a return probe we cannot > capture the formal parameters of the memory function. Actually, we often can. $variables accessed in .function().return context are exactly snapshots of the incoming actual arguments. So for example the trace_kmalloc() case, we could have a k(ret)probes-based fallback based upon inspection of the sources, and unwinding through the "__always_inline" stuff: probe __vm.kmalloc.kp = kernel.function("__kmalloc").return { name = "kmalloc" call_size = 0 caller_function = "" bytes_req = $size bytes_alloc = bytes_req # unavailable gfp_flags = gfp_flag_str($flags) ptr = $return } Based on CONFIG_NUMA (which we can now express preprocessor conditionals on), there may be a _node variant, plus _track_caller variants. All this can be expressed with some effort. - FChE ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] Tracepoint Tapset for Memory Subsystem 2009-10-09 17:38 ` Frank Ch. Eigler @ 2009-10-14 8:32 ` Rajasekhar Duddu 0 siblings, 0 replies; 18+ messages in thread From: Rajasekhar Duddu @ 2009-10-14 8:32 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: systemtap On Fri, Oct 09, 2009 at 01:38:06PM -0400, Frank Ch. Eigler wrote: > Hi - > > On Fri, Oct 09, 2009 at 10:38:05PM +0530, Rajasekhar Duddu wrote: > > [...] > > > > Fallback kprobe is not available for other memory functions because > > > > the variables exported by them are will be modified. > > > > > > Could you elaborate? Do you mean that the same values may not be > > > available from a kprobe context? > > > Yes, the same values may not be available from a kprobe > > context, for example if we take "ret" variable as it is populated mid-way in > > the function and it is also the return value of a function which can > > be captured only by a return probe. But by a return probe we cannot > > capture the formal parameters of the memory function. > > Actually, we often can. $variables accessed in .function().return context > are exactly snapshots of the incoming actual arguments. > > So for example the trace_kmalloc() case, we could have a > k(ret)probes-based fallback based upon inspection of the sources, > and unwinding through the "__always_inline" stuff: > > probe __vm.kmalloc.kp = kernel.function("__kmalloc").return { > name = "kmalloc" > call_size = 0 > caller_function = "" > bytes_req = $size > bytes_alloc = bytes_req # unavailable > gfp_flags = gfp_flag_str($flags) > ptr = $return > } > > Based on CONFIG_NUMA (which we can now express preprocessor > conditionals on), there may be a _node variant, plus _track_caller > variants. All this can be expressed with some effort. > > > - FChE Hi Frank, sure , I will try to have the kprobe based probes with all possible parameters exported in my next patch. Thanks -- Rajasekhar Duddu (rajduddu@linux.vnet.ibm.com), Linux on System z - CSVT, IBM LTC, Bangalore. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2009-10-14 8:32 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-09-19 5:01 [PATCH] Tracepoint Tapset for Memory Subsystem Rajasekhar Duddu 2009-09-22 17:39 ` David Smith 2009-09-22 21:23 ` Frank Ch. Eigler 2009-09-22 22:05 ` David Smith 2009-09-24 18:08 ` [PATCH v2] " Rajasekhar Duddu 2009-09-25 19:19 ` David Smith 2009-09-25 20:07 ` Frank Ch. Eigler 2009-09-28 18:12 ` Jim Keniston 2009-09-29 8:58 ` K.Prasad 2009-09-25 21:50 ` Josh Stone 2009-09-30 10:12 ` Rajasekhar Duddu 2009-10-02 15:14 ` [PATCH v3] " Rajasekhar Duddu 2009-10-06 19:01 ` Frank Ch. Eigler 2009-10-07 13:07 ` Rajasekhar Duddu 2009-10-07 19:51 ` Frank Ch. Eigler 2009-10-09 17:08 ` Rajasekhar Duddu 2009-10-09 17:38 ` Frank Ch. Eigler 2009-10-14 8:32 ` Rajasekhar Duddu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).