* changelog files, %( %) idioms @ 2008-02-25 15:00 Frank Ch. Eigler 2008-03-03 12:18 ` Ananth N Mavinakayanahalli 2008-03-06 9:10 ` examples/wait4time.stp Wenji Huang 0 siblings, 2 replies; 8+ messages in thread From: Frank Ch. Eigler @ 2008-02-25 15:00 UTC (permalink / raw) To: srinivasa; +Cc: systemtap Hi - Please remember to add ChangeLog file entries for patches you commit. You committed several important fixes that only show up on careful search of cvs/git logs. Please rescan src/HACKING for guidelines, and consider retroactively adding the ChangeLog entries. Thanks for the bug #5772 patch. It is possible to make the %( kernel %) conditionals look more compact by realizing that they don't operate at the statement but at the token level. That means one can use them around just the smallest bit of code that needs to be changed for different versions/architectures, so that instead of: %( kernel_vr > "2.6.24" %? argstr = sprintf("%d, %p, %s, %p", $upid, $stat_addr, _wait4_opt_str($options), $ru) %: argstr = sprintf("%d, %p, %s, %p", $pid, $stat_addr, _wait4_opt_str($options), $ru) %) and %( kernel_vr > "2.6.24" %? pid = $upid %: pid = $pid %) one could write ... argstr = sprintf("%d, %p, %s, %p", %( kernel_vr > "2.6.24" %? $upid %: $pid %), $stat_addr, _wait4_opt_str($options), $ru) and pid = %( kernel_vr > "2.6.24" %? $upid %: $pid %) - FChE ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: changelog files, %( %) idioms 2008-02-25 15:00 changelog files, %( %) idioms Frank Ch. Eigler @ 2008-03-03 12:18 ` Ananth N Mavinakayanahalli 2008-03-03 16:24 ` Frank Ch. Eigler 2008-03-06 9:10 ` examples/wait4time.stp Wenji Huang 1 sibling, 1 reply; 8+ messages in thread From: Ananth N Mavinakayanahalli @ 2008-03-03 12:18 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: srinivasa, systemtap On Mon, Feb 25, 2008 at 09:59:15AM -0500, Frank Ch. Eigler wrote: > > Thanks for the bug #5772 patch. It is possible to make the %( kernel > %) conditionals look more compact by realizing that they don't operate > at the statement but at the token level. That means one can use them > around just the smallest bit of code that needs to be changed for > different versions/architectures, so that instead of: > > %( kernel_vr > "2.6.24" %? > argstr = sprintf("%d, %p, %s, %p", $upid, $stat_addr, _wait4_opt_str($options), $ru) > %: > argstr = sprintf("%d, %p, %s, %p", $pid, $stat_addr, _wait4_opt_str($options), $ru) > %) > > and > > %( kernel_vr > "2.6.24" %? > pid = $upid > %: > pid = $pid > %) > > one could write ... > > argstr = sprintf("%d, %p, %s, %p", > %( kernel_vr > "2.6.24" %? $upid %: $pid %), > $stat_addr, _wait4_opt_str($options), $ru) > > and > > pid = %( kernel_vr > "2.6.24" %? $upid %: $pid %) We have a similar issue with the x86_32 syscalls tapset. sys_signalstack has a reference to ebx, while with register unification, its now bx. The tapset is thus broken for kernel_vr > 2.6.24. Following the compact method, the patch looks thus: --- tapset/i686/syscalls.stp | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) Index: systemtap-3mar/tapset/i686/syscalls.stp =================================================================== --- systemtap-3mar.orig/tapset/i686/syscalls.stp +++ systemtap-3mar/tapset/i686/syscalls.stp @@ -119,8 +119,8 @@ probe syscall.set_zone_reclaim.return = # probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { name = "sigaltstack" - ebx = $ebx - argstr = sprintf("%p", $ebx) + bx = %( kernel_vr > "2.6.24" %? $bx %: $ebx %) + argstr = sprintf("%p", %( kernel_vr > "2.6.24" %? $bx %: $ebx %) ) } probe syscall.sigaltstack.return = kernel.function("sys_sigaltstack").return { name = "sigaltstack" Notice that the LHS has changed from ebx to bx. However, if we need to have a 100% backward compatibility, I'd prefer the older approach thus: --- tapset/i686/syscalls.stp | 5 +++++ 1 files changed, 5 insertions(+) Index: systemtap-3mar/tapset/i686/syscalls.stp =================================================================== --- systemtap-3mar.orig/tapset/i686/syscalls.stp +++ systemtap-3mar/tapset/i686/syscalls.stp @@ -119,8 +119,13 @@ probe syscall.set_zone_reclaim.return = # probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { name = "sigaltstack" +%( kernel_vr > "2.6.24" %? + bx = $bx + argstr = sprintf("%p", $bx) +%: ebx = $ebx argstr = sprintf("%p", $ebx) +%) } probe syscall.sigaltstack.return = kernel.function("sys_sigaltstack").return { name = "sigaltstack" Comments? Ananth ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: changelog files, %( %) idioms 2008-03-03 12:18 ` Ananth N Mavinakayanahalli @ 2008-03-03 16:24 ` Frank Ch. Eigler 2008-03-03 16:48 ` William Cohen 0 siblings, 1 reply; 8+ messages in thread From: Frank Ch. Eigler @ 2008-03-03 16:24 UTC (permalink / raw) To: Ananth N Mavinakayanahalli; +Cc: srinivasa, systemtap Hi - On Mon, Mar 03, 2008 at 05:48:41PM +0530, Ananth N Mavinakayanahalli wrote: > [...] > probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { > name = "sigaltstack" > - ebx = $ebx > - argstr = sprintf("%p", $ebx) > + bx = %( kernel_vr > "2.6.24" %? $bx %: $ebx %) > + argstr = sprintf("%p", %( kernel_vr > "2.6.24" %? $bx %: $ebx %) ) I suggest picking a single more informative variable name than "ebx" or "bx" for that parameter. Then that variable could be used as the plain sprintf value. - FChE ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: changelog files, %( %) idioms 2008-03-03 16:24 ` Frank Ch. Eigler @ 2008-03-03 16:48 ` William Cohen 2008-03-04 4:29 ` Ananth N Mavinakayanahalli 0 siblings, 1 reply; 8+ messages in thread From: William Cohen @ 2008-03-03 16:48 UTC (permalink / raw) To: Frank Ch. Eigler; +Cc: Ananth N Mavinakayanahalli, srinivasa, systemtap Frank Ch. Eigler wrote: > Hi - > > On Mon, Mar 03, 2008 at 05:48:41PM +0530, Ananth N Mavinakayanahalli wrote: >> [...] >> probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { >> name = "sigaltstack" >> - ebx = $ebx >> - argstr = sprintf("%p", $ebx) >> + bx = %( kernel_vr > "2.6.24" %? $bx %: $ebx %) >> + argstr = sprintf("%p", %( kernel_vr > "2.6.24" %? $bx %: $ebx %) ) > > I suggest picking a single more informative variable name than "ebx" > or "bx" for that parameter. Then that variable could be used as the > plain sprintf value. > > - FChE I saw this this cause failure on F-9 i686. There could be 2.6.24.1, so probably want to check that the kernel is earlier than 2.6.25. Something like the following: bx = %( kernel_vr < "2.6.25" %? $bx %: $ebx %) Could the use of $bx be factored out of argstr, so there is only one check for the kernel version for $ebx/$bx rather than two? -Will ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: changelog files, %( %) idioms 2008-03-03 16:48 ` William Cohen @ 2008-03-04 4:29 ` Ananth N Mavinakayanahalli 2008-03-06 14:48 ` William Cohen 0 siblings, 1 reply; 8+ messages in thread From: Ananth N Mavinakayanahalli @ 2008-03-04 4:29 UTC (permalink / raw) To: William Cohen; +Cc: Frank Ch. Eigler, srinivasa, systemtap On Mon, Mar 03, 2008 at 11:48:13AM -0500, William Cohen wrote: > Frank Ch. Eigler wrote: >> Hi - >> >> On Mon, Mar 03, 2008 at 05:48:41PM +0530, Ananth N Mavinakayanahalli wrote: >>> [...] >>> probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { >>> name = "sigaltstack" >>> - ebx = $ebx >>> - argstr = sprintf("%p", $ebx) >>> + bx = %( kernel_vr > "2.6.24" %? $bx %: $ebx %) >>> + argstr = sprintf("%p", %( kernel_vr > "2.6.24" %? $bx %: $ebx %) ) >> >> I suggest picking a single more informative variable name than "ebx" >> or "bx" for that parameter. Then that variable could be used as the >> plain sprintf value. >> >> - FChE > > I saw this this cause failure on F-9 i686. There could be 2.6.24.1, so > probably want to check that the kernel is earlier than 2.6.25. Something > like the following: Actually, this went into 2.6.25-rc > bx = %( kernel_vr < "2.6.25" %? $bx %: $ebx %) > > Could the use of $bx be factored out of argstr, so there is only one check > for the kernel version for $ebx/$bx rather than two? How does this look? --- tapset/i686/syscalls.stp | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) Index: systemtap-4mar/tapset/i686/syscalls.stp =================================================================== --- systemtap-4mar.orig/tapset/i686/syscalls.stp +++ systemtap-4mar/tapset/i686/syscalls.stp @@ -119,8 +119,8 @@ probe syscall.set_zone_reclaim.return = # probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { name = "sigaltstack" - ebx = $ebx - argstr = sprintf("%p", $ebx) + ussp = %( kernel_vr < "2.6.25" %? $ebx %: $bx %) + argstr = sprintf("%p", ussp) } probe syscall.sigaltstack.return = kernel.function("sys_sigaltstack").return { name = "sigaltstack" ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: changelog files, %( %) idioms 2008-03-04 4:29 ` Ananth N Mavinakayanahalli @ 2008-03-06 14:48 ` William Cohen 2008-03-06 15:16 ` Ananth N Mavinakayanahalli 0 siblings, 1 reply; 8+ messages in thread From: William Cohen @ 2008-03-06 14:48 UTC (permalink / raw) To: ananth; +Cc: Frank Ch. Eigler, srinivasa, systemtap Ananth N Mavinakayanahalli wrote: > On Mon, Mar 03, 2008 at 11:48:13AM -0500, William Cohen wrote: >> Frank Ch. Eigler wrote: >>> Hi - >>> >>> On Mon, Mar 03, 2008 at 05:48:41PM +0530, Ananth N Mavinakayanahalli wrote: >>>> [...] >>>> probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { >>>> name = "sigaltstack" >>>> - ebx = $ebx >>>> - argstr = sprintf("%p", $ebx) >>>> + bx = %( kernel_vr > "2.6.24" %? $bx %: $ebx %) >>>> + argstr = sprintf("%p", %( kernel_vr > "2.6.24" %? $bx %: $ebx %) ) >>> I suggest picking a single more informative variable name than "ebx" >>> or "bx" for that parameter. Then that variable could be used as the >>> plain sprintf value. >>> >>> - FChE >> I saw this this cause failure on F-9 i686. There could be 2.6.24.1, so >> probably want to check that the kernel is earlier than 2.6.25. Something >> like the following: > > Actually, this went into 2.6.25-rc > >> bx = %( kernel_vr < "2.6.25" %? $bx %: $ebx %) >> >> Could the use of $bx be factored out of argstr, so there is only one check >> for the kernel version for $ebx/$bx rather than two? > > How does this look? > > --- > tapset/i686/syscalls.stp | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > Index: systemtap-4mar/tapset/i686/syscalls.stp > =================================================================== > --- systemtap-4mar.orig/tapset/i686/syscalls.stp > +++ systemtap-4mar/tapset/i686/syscalls.stp > @@ -119,8 +119,8 @@ probe syscall.set_zone_reclaim.return = > # > probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { > name = "sigaltstack" > - ebx = $ebx > - argstr = sprintf("%p", $ebx) > + ussp = %( kernel_vr < "2.6.25" %? $ebx %: $bx %) > + argstr = sprintf("%p", ussp) > } > probe syscall.sigaltstack.return = kernel.function("sys_sigaltstack").return { > name = "sigaltstack" That looks okay. -Will ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: changelog files, %( %) idioms 2008-03-06 14:48 ` William Cohen @ 2008-03-06 15:16 ` Ananth N Mavinakayanahalli 0 siblings, 0 replies; 8+ messages in thread From: Ananth N Mavinakayanahalli @ 2008-03-06 15:16 UTC (permalink / raw) To: William Cohen; +Cc: Frank Ch. Eigler, srinivasa, systemtap On Thu, Mar 06, 2008 at 09:48:30AM -0500, William Cohen wrote: > Ananth N Mavinakayanahalli wrote: >> On Mon, Mar 03, 2008 at 11:48:13AM -0500, William Cohen wrote: >>> Frank Ch. Eigler wrote: >>>> Hi - >>>> >>>> On Mon, Mar 03, 2008 at 05:48:41PM +0530, Ananth N Mavinakayanahalli wrote: >>>>> [...] >>>>> probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { >>>>> name = "sigaltstack" >>>>> - ebx = $ebx >>>>> - argstr = sprintf("%p", $ebx) >>>>> + bx = %( kernel_vr > "2.6.24" %? $bx %: $ebx %) >>>>> + argstr = sprintf("%p", %( kernel_vr > "2.6.24" %? $bx %: $ebx %) ) >>>> I suggest picking a single more informative variable name than "ebx" >>>> or "bx" for that parameter. Then that variable could be used as the >>>> plain sprintf value. >>>> >>>> - FChE >>> I saw this this cause failure on F-9 i686. There could be 2.6.24.1, so >>> probably want to check that the kernel is earlier than 2.6.25. Something >>> like the following: >> >> Actually, this went into 2.6.25-rc >> >>> bx = %( kernel_vr < "2.6.25" %? $bx %: $ebx %) >>> >>> Could the use of $bx be factored out of argstr, so there is only one >>> check for the kernel version for $ebx/$bx rather than two? >> >> How does this look? >> >> --- >> tapset/i686/syscalls.stp | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> Index: systemtap-4mar/tapset/i686/syscalls.stp >> =================================================================== >> --- systemtap-4mar.orig/tapset/i686/syscalls.stp >> +++ systemtap-4mar/tapset/i686/syscalls.stp >> @@ -119,8 +119,8 @@ probe syscall.set_zone_reclaim.return = >> # >> probe syscall.sigaltstack = kernel.function("sys_sigaltstack") { >> name = "sigaltstack" >> - ebx = $ebx >> - argstr = sprintf("%p", $ebx) >> + ussp = %( kernel_vr < "2.6.25" %? $ebx %: $bx %) >> + argstr = sprintf("%p", ussp) >> } >> probe syscall.sigaltstack.return = kernel.function("sys_sigaltstack").return { >> name = "sigaltstack" > > That looks okay. Patch committed. Thanks, Ananth ^ permalink raw reply [flat|nested] 8+ messages in thread
* examples/wait4time.stp 2008-02-25 15:00 changelog files, %( %) idioms Frank Ch. Eigler 2008-03-03 12:18 ` Ananth N Mavinakayanahalli @ 2008-03-06 9:10 ` Wenji Huang 1 sibling, 0 replies; 8+ messages in thread From: Wenji Huang @ 2008-03-06 9:10 UTC (permalink / raw) To: systemtap Hi, In previous threads, there is the discussion about syscall.wait4 due to renamed parameter in kernel 2.6.25. pid = %( kernel_vr > "2.6.24" %? $upid %: $pid %) So the related script examples/wait4time.stp need to reflect that. t = gettimeofday_us(); p = pid() entry_wait4[p] = t - wait4_pid[p]=$pid + wait4_pid[p]=pid } Regards, Wenji ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-03-06 15:16 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-02-25 15:00 changelog files, %( %) idioms Frank Ch. Eigler 2008-03-03 12:18 ` Ananth N Mavinakayanahalli 2008-03-03 16:24 ` Frank Ch. Eigler 2008-03-03 16:48 ` William Cohen 2008-03-04 4:29 ` Ananth N Mavinakayanahalli 2008-03-06 14:48 ` William Cohen 2008-03-06 15:16 ` Ananth N Mavinakayanahalli 2008-03-06 9:10 ` examples/wait4time.stp Wenji Huang
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).