Patchwork [RFC] UBUNTU: fix perf kernel version detection for multiple-flavour strings

login
register
mail settings
Submitter Tim Gardner
Date April 21, 2010, 1:25 p.m.
Message ID <4BCEFCE6.2070506@canonical.com>
Download mbox | patch
Permalink /patch/50656/
State Superseded
Delegated to: Andy Whitcroft
Headers show

Comments

Tim Gardner - April 21, 2010, 1:25 p.m.
On 04/21/2010 06:42 AM, Jeremy Kerr wrote:
> Currently, the perf tool doesn't work for the generic-pae flavour:
>
>   $ bash -x /usr/bin/perf
>   ++ uname -r
>   + version=2.6.32-21-generic-pae
>   + version=2.6.32-21-generic
>   + exec perf_2.6.32-21-generic
>   /usr/bin/perf: line 4: exec: perf_2.6.32-21-generic: not found
>
> - the PAE flavour has a version string ending in -generic-pae, but the
> version cleaning in the perf script doesn't handle the multiple flavour
> strings correctly. The perf tool is named perf_2.6.32-21, not
> perf_2.6.32-21-generic.
>
> This change fixes the perf wrapper script to throw away version data
> after a 'dash, non-digit' sequence instead of just the last dash. This
> fixes the problem on the PAE kernel. We need to do a special pass for
> the -386 flavour, as it's virtually indisinguishable from a normal
> version number. Testing this parsing against the possible flavours
> gives:
>
>          2.6.32-21-generic ->  2.6.32-21
>           2.6.32-21-server ->  2.6.32-21
>          2.6.32-21-preempt ->  2.6.32-21
>        2.6.32-21-versatile ->  2.6.32-21
>          2.6.32-21-generic ->  2.6.32-21
>      2.6.32-21-generic-pae ->  2.6.32-21
>              2.6.32-21-386 ->  2.6.32-21
>             2.6.32-21-ia64 ->  2.6.32-21
>             2.6.32-21-lpia ->  2.6.32-21
>          2.6.32-21-powerpc ->  2.6.32-21
>      2.6.32-21-powerpc-smp ->  2.6.32-21
>    2.6.32-21-powerpc64-smp ->  2.6.32-21
>          2.6.32-21-sparc64 ->  2.6.32-21
>      2.6.32-21-sparc64-smp ->  2.6.32-21
>
> Signed-off-by: Jeremy Kerr<jeremy.kerr@canonical.com>
>
> ---
>   debian/tools/perf |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/debian/tools/perf b/debian/tools/perf
> index 1a9915f..33df59d 100644
> --- a/debian/tools/perf
> +++ b/debian/tools/perf
> @@ -1,4 +1,5 @@
>   #!/bin/bash
>   version=`uname -r`
> -version=${version%-*}
> +version=${version/-[^0-9]*}
> +version=${version%-386}
>   exec "perf_$version" "$@"
>

Why not just drop everything after the second '-' inclusive ?

rtg
Andy Whitcroft - April 21, 2010, 1:30 p.m.
On Wed, Apr 21, 2010 at 07:25:58AM -0600, Tim Gardner wrote:
> On 04/21/2010 06:42 AM, Jeremy Kerr wrote:
> >Currently, the perf tool doesn't work for the generic-pae flavour:
> >
> >  $ bash -x /usr/bin/perf
> >  ++ uname -r
> >  + version=2.6.32-21-generic-pae
> >  + version=2.6.32-21-generic
> >  + exec perf_2.6.32-21-generic
> >  /usr/bin/perf: line 4: exec: perf_2.6.32-21-generic: not found
> >
> >- the PAE flavour has a version string ending in -generic-pae, but the
> >version cleaning in the perf script doesn't handle the multiple flavour
> >strings correctly. The perf tool is named perf_2.6.32-21, not
> >perf_2.6.32-21-generic.
> >
> >This change fixes the perf wrapper script to throw away version data
> >after a 'dash, non-digit' sequence instead of just the last dash. This
> >fixes the problem on the PAE kernel. We need to do a special pass for
> >the -386 flavour, as it's virtually indisinguishable from a normal
> >version number. Testing this parsing against the possible flavours
> >gives:
> >
> >         2.6.32-21-generic ->  2.6.32-21
> >          2.6.32-21-server ->  2.6.32-21
> >         2.6.32-21-preempt ->  2.6.32-21
> >       2.6.32-21-versatile ->  2.6.32-21
> >         2.6.32-21-generic ->  2.6.32-21
> >     2.6.32-21-generic-pae ->  2.6.32-21
> >             2.6.32-21-386 ->  2.6.32-21
> >            2.6.32-21-ia64 ->  2.6.32-21
> >            2.6.32-21-lpia ->  2.6.32-21
> >         2.6.32-21-powerpc ->  2.6.32-21
> >     2.6.32-21-powerpc-smp ->  2.6.32-21
> >   2.6.32-21-powerpc64-smp ->  2.6.32-21
> >         2.6.32-21-sparc64 ->  2.6.32-21
> >     2.6.32-21-sparc64-smp ->  2.6.32-21
> >
> >Signed-off-by: Jeremy Kerr<jeremy.kerr@canonical.com>
> >
> >---
> >  debian/tools/perf |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/debian/tools/perf b/debian/tools/perf
> >index 1a9915f..33df59d 100644
> >--- a/debian/tools/perf
> >+++ b/debian/tools/perf
> >@@ -1,4 +1,5 @@
> >  #!/bin/bash
> >  version=`uname -r`
> >-version=${version%-*}
> >+version=${version/-[^0-9]*}
> >+version=${version%-386}
> >  exec "perf_$version" "$@"
> >
> 
> Why not just drop everything after the second '-' inclusive ?
> 
> rtg
> -- 
> Tim Gardner tim.gardner@canonical.com

> From f935a6337ea4a1b8d7061fc249cbbfa13c38f631 Mon Sep 17 00:00:00 2001
> From: Tim Gardner <tim.gardner@canonical.com>
> Date: Wed, 21 Apr 2010 07:24:13 -0600
> Subject: [PATCH] UBUNTU: Do a better job of stripping version information from the perf binary name
> 
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
>  debian/tools/perf |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/debian/tools/perf b/debian/tools/perf
> index 1a9915f..7a3c10b 100644
> --- a/debian/tools/perf
> +++ b/debian/tools/perf
> @@ -1,4 +1,3 @@
>  #!/bin/bash
> -version=`uname -r`
> -version=${version%-*}
> +version=`uname -r|sed 's/\([0-9]*\)\.\([0-9]*\)\.\([0-9]*\)-\([0-9]*\)-.*$/\1\.\2\.\3-\4/'`
>  exec "perf_$version" "$@"

Yes we should use the known bits here.  I would prefer to avoid the
additional fork here if possible.  I think we can do that, see my other
reply for an alternative patch.

-apw
Tim Gardner - April 21, 2010, 1:42 p.m.
On 04/21/2010 07:30 AM, Andy Whitcroft wrote:
> On Wed, Apr 21, 2010 at 07:25:58AM -0600, Tim Gardner wrote:
>> On 04/21/2010 06:42 AM, Jeremy Kerr wrote:
>>> Currently, the perf tool doesn't work for the generic-pae flavour:
>>>
>>>   $ bash -x /usr/bin/perf
>>>   ++ uname -r
>>>   + version=2.6.32-21-generic-pae
>>>   + version=2.6.32-21-generic
>>>   + exec perf_2.6.32-21-generic
>>>   /usr/bin/perf: line 4: exec: perf_2.6.32-21-generic: not found
>>>
>>> - the PAE flavour has a version string ending in -generic-pae, but the
>>> version cleaning in the perf script doesn't handle the multiple flavour
>>> strings correctly. The perf tool is named perf_2.6.32-21, not
>>> perf_2.6.32-21-generic.
>>>
>>> This change fixes the perf wrapper script to throw away version data
>>> after a 'dash, non-digit' sequence instead of just the last dash. This
>>> fixes the problem on the PAE kernel. We need to do a special pass for
>>> the -386 flavour, as it's virtually indisinguishable from a normal
>>> version number. Testing this parsing against the possible flavours
>>> gives:
>>>
>>>          2.6.32-21-generic ->   2.6.32-21
>>>           2.6.32-21-server ->   2.6.32-21
>>>          2.6.32-21-preempt ->   2.6.32-21
>>>        2.6.32-21-versatile ->   2.6.32-21
>>>          2.6.32-21-generic ->   2.6.32-21
>>>      2.6.32-21-generic-pae ->   2.6.32-21
>>>              2.6.32-21-386 ->   2.6.32-21
>>>             2.6.32-21-ia64 ->   2.6.32-21
>>>             2.6.32-21-lpia ->   2.6.32-21
>>>          2.6.32-21-powerpc ->   2.6.32-21
>>>      2.6.32-21-powerpc-smp ->   2.6.32-21
>>>    2.6.32-21-powerpc64-smp ->   2.6.32-21
>>>          2.6.32-21-sparc64 ->   2.6.32-21
>>>      2.6.32-21-sparc64-smp ->   2.6.32-21
>>>
>>> Signed-off-by: Jeremy Kerr<jeremy.kerr@canonical.com>
>>>
>>> ---
>>>   debian/tools/perf |    3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/debian/tools/perf b/debian/tools/perf
>>> index 1a9915f..33df59d 100644
>>> --- a/debian/tools/perf
>>> +++ b/debian/tools/perf
>>> @@ -1,4 +1,5 @@
>>>   #!/bin/bash
>>>   version=`uname -r`
>>> -version=${version%-*}
>>> +version=${version/-[^0-9]*}
>>> +version=${version%-386}
>>>   exec "perf_$version" "$@"
>>>
>>
>> Why not just drop everything after the second '-' inclusive ?
>>
>> rtg
>> --
>> Tim Gardner tim.gardner@canonical.com
>
>>  From f935a6337ea4a1b8d7061fc249cbbfa13c38f631 Mon Sep 17 00:00:00 2001
>> From: Tim Gardner<tim.gardner@canonical.com>
>> Date: Wed, 21 Apr 2010 07:24:13 -0600
>> Subject: [PATCH] UBUNTU: Do a better job of stripping version information from the perf binary name
>>
>> Signed-off-by: Tim Gardner<tim.gardner@canonical.com>
>> ---
>>   debian/tools/perf |    3 +--
>>   1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/debian/tools/perf b/debian/tools/perf
>> index 1a9915f..7a3c10b 100644
>> --- a/debian/tools/perf
>> +++ b/debian/tools/perf
>> @@ -1,4 +1,3 @@
>>   #!/bin/bash
>> -version=`uname -r`
>> -version=${version%-*}
>> +version=`uname -r|sed 's/\([0-9]*\)\.\([0-9]*\)\.\([0-9]*\)-\([0-9]*\)-.*$/\1\.\2\.\3-\4/'`
>>   exec "perf_$version" "$@"
>
> Yes we should use the known bits here.  I would prefer to avoid the
> additional fork here if possible.  I think we can do that, see my other
> reply for an alternative patch.
>
> -apw

meh. I have no strong opinion.

Patch

From f935a6337ea4a1b8d7061fc249cbbfa13c38f631 Mon Sep 17 00:00:00 2001
From: Tim Gardner <tim.gardner@canonical.com>
Date: Wed, 21 Apr 2010 07:24:13 -0600
Subject: [PATCH] UBUNTU: Do a better job of stripping version information from the perf binary name

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 debian/tools/perf |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/debian/tools/perf b/debian/tools/perf
index 1a9915f..7a3c10b 100644
--- a/debian/tools/perf
+++ b/debian/tools/perf
@@ -1,4 +1,3 @@ 
 #!/bin/bash
-version=`uname -r`
-version=${version%-*}
+version=`uname -r|sed 's/\([0-9]*\)\.\([0-9]*\)\.\([0-9]*\)-\([0-9]*\)-.*$/\1\.\2\.\3-\4/'`
 exec "perf_$version" "$@"
-- 
1.7.0.4