From 6e2ed59997315467d4426969829b83d4b573016e Mon Sep 17 00:00:00 2001 From: "Frank Ch. Eigler" Date: Tue, 6 Oct 2015 16:41:02 -0400 Subject: [PATCH] RHBZ1269062 redux, less quote elevator_name dsmith pointed out that switching to a quoted string is not always the right thing to do, esp. from a backward compatibility point of view. We now formalize a policy guideline in DEVGUIDE that *_string2 be used when pointers should be valid but might be 0, which is the situation with these probe points. --- NEWS | 3 --- tapset/DEVGUIDE | 14 ++++++++++++++ tapset/linux/ioscheduler.stp | 32 ++++++++++++++++---------------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/NEWS b/NEWS index 82ed80748..726201264 100644 --- a/NEWS +++ b/NEWS @@ -1,8 +1,5 @@ * What's new in version 2.9, 2015-09-30 -- The ioscheduler.* tapset functions return a quoted elevator_name string - rather than unquoted, since the underlying kernel pointer could be null. - - SystemTap now uses symbols from /proc/kallsyms when kernel debuginfo is not available. diff --git a/tapset/DEVGUIDE b/tapset/DEVGUIDE index 2e3a6bd4c..570e1ff84 100644 --- a/tapset/DEVGUIDE +++ b/tapset/DEVGUIDE @@ -81,6 +81,14 @@ SystemTap language, like access structure fields in some contexts, follow linked lists, etc. You can use auxiliary functions defined in other tapsets or write your own. +Take care when target variables may have invalid, unavailable, or +esoteric values, or are passed via pointers. The a $pointer->field +dereference can fail, so consider wrapping a try { } catch { } around +it. For a char* being converted to a string, use {kernel,user}_string +only if the pointer is guaranteed valid. Use {kernel,user}_string2($ptr,"") +if the pointer might be NULL but should otherwise be good. Use +{kernel,user}_string_quoted($ptr) to handle any possible pointer. + In the following example, copy_process() returns a pointer to the task_struct for the new process. Note that the process ID of the new process is retrieved by calling task_pid() and passing it the task_struct @@ -278,6 +286,12 @@ into the body of the embedded-C function, to mark it as /* unprivileged */ (see above) and to have the systemtap translator automatically add a call to assert_is_myproc() to the beginning of the function. +Add the string + /* unmodified-fnargs */ +into the body of the embedded-C function to inform the translator +that any string inputs and return values may be passed by reference +rather than copy. This enables optimization. + Add the string /* unmangled */ into a legacy (pre-SystemTap 1.8) embedded-C function that needs to diff --git a/tapset/linux/ioscheduler.stp b/tapset/linux/ioscheduler.stp index 07562ff1f..8da4a4c0f 100644 --- a/tapset/linux/ioscheduler.stp +++ b/tapset/linux/ioscheduler.stp @@ -23,10 +23,10 @@ probe ioscheduler.elv_next_request = kernel.function("blk_peek_request") !, kernel.function("elv_next_request") { name = "elv_next_request" - elevator_name = kernel_string_quoted( + elevator_name = kernel_string2( @choose_defined($q->elevator->type->elevator_name, @choose_defined($q->elevator->elevator_type->elevator_name, - $q->elevator->elevator_name))) + $q->elevator->elevator_name)), "") } /** @@ -73,10 +73,10 @@ probe ioscheduler.elv_completed_request = kernel.function("elv_completed_request") { name = "elv_completed_request" - elevator_name = kernel_string_quoted( + elevator_name = kernel_string2( @choose_defined($q->elevator->type->elevator_name, @choose_defined($q->elevator->elevator_type->elevator_name, - $q->elevator->elevator_name))) + $q->elevator->elevator_name)), "") if($rq == 0) { disk_major = -1 disk_minor = -1 @@ -111,10 +111,10 @@ probe ioscheduler.elv_add_request.kp = kernel.function("__elv_add_request") { name = "elv_add_request" - elevator_name = kernel_string_quoted( + elevator_name = kernel_string2( @choose_defined($q->elevator->type->elevator_name, @choose_defined($q->elevator->elevator_type->elevator_name, - $q->elevator->elevator_name))) + $q->elevator->elevator_name)), "") q = $q if($rq == 0) { disk_major = -1 @@ -149,10 +149,10 @@ probe ioscheduler.elv_add_request.tp = kernel.trace("block_rq_insert") ? { name = "elv_add_request" q = $q - elevator_name = kernel_string_quoted( + elevator_name = kernel_string2( @choose_defined($q->elevator->type->elevator_name, @choose_defined($q->elevator->elevator_type->elevator_name, - $q->elevator->elevator_name))) + $q->elevator->elevator_name)), "") rq = $rq if ($rq == 0 || $rq->rq_disk ==0) { @@ -197,10 +197,10 @@ probe ioscheduler_trace.elv_completed_request = kernel.trace("block_rq_complete") ? { name = "elv_completed_request" - elevator_name = kernel_string_quoted( + elevator_name = kernel_string2( @choose_defined($q->elevator->type->elevator_name, @choose_defined($q->elevator->elevator_type->elevator_name, - $q->elevator->elevator_name))) + $q->elevator->elevator_name)), "") rq = $rq if ($rq == 0 || $rq->rq_disk ==0) { @@ -231,10 +231,10 @@ probe ioscheduler_trace.elv_issue_request = kernel.trace("block_rq_issue") ? { name = "elv_issue_request" - elevator_name = kernel_string_quoted( + elevator_name = kernel_string2( @choose_defined($q->elevator->type->elevator_name, @choose_defined($q->elevator->elevator_type->elevator_name, - $q->elevator->elevator_name))) + $q->elevator->elevator_name)), "") rq = $rq if ($rq == 0 || $rq->rq_disk ==0) { @@ -265,10 +265,10 @@ probe ioscheduler_trace.elv_requeue_request = kernel.trace("block_rq_requeue") ? { name = "elv_requeue_request" - elevator_name = kernel_string_quoted( + elevator_name = kernel_string2( @choose_defined($q->elevator->type->elevator_name, @choose_defined($q->elevator->elevator_type->elevator_name, - $q->elevator->elevator_name))) + $q->elevator->elevator_name)), "") rq = $rq if ($rq == 0 || $rq->rq_disk ==0) { @@ -298,10 +298,10 @@ probe ioscheduler_trace.elv_abort_request = kernel.trace("block_rq_abort") ? { name = "elv_abort_request" - elevator_name = kernel_string_quoted( + elevator_name = kernel_string2( @choose_defined($q->elevator->type->elevator_name, @choose_defined($q->elevator->elevator_type->elevator_name, - $q->elevator->elevator_name))) + $q->elevator->elevator_name)), "") rq = $rq if ($rq == 0 || $rq->rq_disk ==0) { -- 2.43.5