Re: perf script: rwtop: SIGALRM and pipe read race

[Andrew Jones]

On Tue, Sep 18, 2012 at 11:05:42AM +0200, Andrew Jones wrote:
> Please read the link I posted to the Perl documentation. The standard
> Perl signal handling doesn't support SA_RESTART. The Perl developers
> found it could lead to data corruption. The patch above attempts to
> replace the standard signal handler with a safe one that supports
> SA_RESTART. It has nothing to do with the display routine, and
> everything to do with avoiding the need for your EINTR patch.

Oh, I should state that it *does* avoid your EINTR patch. With the -D
added to the record script, then besides the unsigned $ret issue,
rwtop works fine for me now with with this SA_RESTART patch.

Below is a v2 where I've also updated the minimum version of Perl
needed. My understanding is that < 5.8.0 $SIG might work fine.
>= 5.8.0 it won't, and 5.8.2 or later is needed for ->safe().

diff --git a/tools/perf/scripts/perl/rwtop.pl b/tools/perf/scripts/perl/rwtop.pl
index 4bb3ecd..617a4d5 100644
--- a/tools/perf/scripts/perl/rwtop.pl
+++ b/tools/perf/scripts/perl/rwtop.pl
@@ -9,7 +9,7 @@
 # refreshed every [interval] seconds.  The default interval is 3
 # seconds.
 
-use 5.010000;
+use 5.8.2;
 use strict;
 use warnings;
 
@@ -17,6 +17,7 @@ use lib "$ENV{'PERF_EXEC_PATH'}/scripts/perl/Perf-Trace-Util/lib";
 use lib "./Perf-Trace-Util/lib";
 use Perf::Trace::Core;
 use Perf::Trace::Util;
+use POSIX qw/SIGALRM SA_RESTART/;
 
 my $default_interval = 3;
 my $nlines = 20;
@@ -90,7 +91,10 @@ sub syscalls::sys_enter_write
 
 sub trace_begin
 {
-    $SIG{ALRM} = \&set_print_pending;
+    my $sa = POSIX::SigAction->new(\&set_print_pending);
+    $sa->flags(SA_RESTART);
+    $sa->safe(1);
+    POSIX::sigaction(SIGALRM, $sa) or die "Can't set SIGALRM handler: $!\n";
     alarm 1;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/