linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] RFC perf test 14 add platform dependency
@ 2017-06-13  9:09 Thomas Richter
  2017-06-16 16:08 ` Arnaldo Carvalho de Melo
  2017-06-19 20:59 ` Jiri Olsa
  0 siblings, 2 replies; 9+ messages in thread
From: Thomas Richter @ 2017-06-13  9:09 UTC (permalink / raw)
  To: jolsa, linux-perf-users; +Cc: Thomas Richter

This is a proposal to add platform dependency into the
test case 14 (perf_event_attr). It is based on a suggestion from
Jiri Olsa.
Add a new optional attribute named 'machine' in the [config] section
of the test case file. It is a comma separated list of architecture
names this test can be executed on. For example:

machine = x86_64,alpha,ppc

If this attribute is missing the test is executed on any platform.
This does not break the current setup.
The values listed for this attribute should be identical to
uname -m output.
If the list starts with an exclamation mark (!) the comparison is
inverted, for example for

machine = !s390x,ppc

the test is not executed on s390x or ppc platforms.
The exclamation mark must be at the beginnning of the list.

Here is an example debug output:
[root@s35lp76]# fgrep machine tests/attr/test-stat-C2
machine = x86_64,alpha,ppc
[root@s35lp76]# PERF_TEST_ATTR=/tmp /usr/bin/python2 ./tests/attr.py \
  -d ./tests/attr/ -p ./perf -vvvvv -t test-stat-C1

provides the following output:

running './tests/attr//test-stat-C1'
test executed only on 'x86_64,alpha,ppc' <--- new
  loading expected events
    Event event:base-stat
      fd = 1
      group_fd = -1
      .....
skipped [s390x] './tests/attr//test-stat-C1' <--- new

The test is skipped with return code 0.

Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
---
 tools/perf/tests/attr.py | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tools/perf/tests/attr.py b/tools/perf/tests/attr.py
index d0242d8..6e71d52 100644
--- a/tools/perf/tests/attr.py
+++ b/tools/perf/tests/attr.py
@@ -30,6 +30,13 @@ class Fail(Exception):
     def getMsg(self):
         return '\'%s\' - %s' % (self.test.path, self.msg)
 
+class Notest(Exception):
+    def __init__(self, test, arch):
+        self.arch = arch
+        self.test = test
+    def getMsg(self):
+        return '[%s] \'%s\'' % (self.arch, self.test.path)
+
 class Unsup(Exception):
     def __init__(self, test):
         self.test = test
@@ -112,6 +119,7 @@ class Event(dict):
 #     'command' - perf command name
 #     'args'    - special command arguments
 #     'ret'     - expected command return value (0 by default)
+#     'machine' - machine specific test
 #
 # [eventX:base]
 #   - one or multiple instances in file
@@ -134,6 +142,12 @@ class Test(object):
         except:
             self.ret  = 0
 
+        try:
+            self.machine  = parser.get('config', 'machine')
+            log.warning("test limitation '%s'" % self.machine)
+        except:
+            self.machine  = ''
+
         self.expect   = {}
         self.result   = {}
         log.debug("  loading expected events");
@@ -145,6 +159,30 @@ class Test(object):
         else:
             return True
 
+    def skip_test(self, arch):
+        # If architecture not set always run test
+        if self.machine == '':
+            return False
+
+        # Allow multiple values in assignment separated by ','
+        machine_list = self.machine.split(',')
+
+        # Handle negated list such as !s390x,ppc
+        if machine_list[0][0] == '!':
+            machine_list[0] = machine_list[0][1:]
+            log.warning("excluded machine_list %s" % machine_list)
+            for machine_item in machine_list:
+                # log.warning("test for %s arch is %s" % (machine_item, arch))
+                if machine_item == arch:
+                    return True
+            return False
+
+        for machine_item in machine_list:
+            # log.warning("test for %s arch is %s" % (machine_item, arch))
+            if machine_item == arch:
+                return False
+        return True
+
     def load_events(self, path, events):
         parser_event = ConfigParser.SafeConfigParser()
         parser_event.read(path)
@@ -168,6 +206,11 @@ class Test(object):
             events[section] = e
 
     def run_cmd(self, tempdir):
+        junk1, junk2, junk3, junk4, arch = (os.uname())
+
+        if self.skip_test(arch):
+            raise Notest(self, arch)
+
         cmd = "PERF_TEST_ATTR=%s %s %s -o %s/perf.data %s" % (tempdir,
               self.perf, self.command, tempdir, self.args)
         ret = os.WEXITSTATUS(os.system(cmd))
@@ -267,6 +310,8 @@ def run_tests(options):
             Test(f, options).run()
         except Unsup, obj:
             log.warning("unsupp  %s" % obj.getMsg())
+        except Notest, obj:
+            log.warning("skipped %s" % obj.getMsg())
 
 def setup_log(verbose):
     global log
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RFC perf test 14 add platform dependency
  2017-06-13  9:09 [PATCH v2] RFC perf test 14 add platform dependency Thomas Richter
@ 2017-06-16 16:08 ` Arnaldo Carvalho de Melo
  2017-06-18 22:51   ` Jiri Olsa
  2017-06-19 20:59 ` Jiri Olsa
  1 sibling, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-06-16 16:08 UTC (permalink / raw)
  To: Thomas Richter; +Cc: jolsa, linux-perf-users

Em Tue, Jun 13, 2017 at 11:09:37AM +0200, Thomas Richter escreveu:
> This is a proposal to add platform dependency into the
> test case 14 (perf_event_attr). It is based on a suggestion from
> Jiri Olsa.
> Add a new optional attribute named 'machine' in the [config] section
> of the test case file. It is a comma separated list of architecture
> names this test can be executed on. For example:
> 
> machine = x86_64,alpha,ppc

Looks ok, Jiri?

- Arnaldo
 
> If this attribute is missing the test is executed on any platform.
> This does not break the current setup.
> The values listed for this attribute should be identical to
> uname -m output.
> If the list starts with an exclamation mark (!) the comparison is
> inverted, for example for
> 
> machine = !s390x,ppc
> 
> the test is not executed on s390x or ppc platforms.
> The exclamation mark must be at the beginnning of the list.
> 
> Here is an example debug output:
> [root@s35lp76]# fgrep machine tests/attr/test-stat-C2
> machine = x86_64,alpha,ppc
> [root@s35lp76]# PERF_TEST_ATTR=/tmp /usr/bin/python2 ./tests/attr.py \
>   -d ./tests/attr/ -p ./perf -vvvvv -t test-stat-C1
> 
> provides the following output:
> 
> running './tests/attr//test-stat-C1'
> test executed only on 'x86_64,alpha,ppc' <--- new
>   loading expected events
>     Event event:base-stat
>       fd = 1
>       group_fd = -1
>       .....
> skipped [s390x] './tests/attr//test-stat-C1' <--- new
> 
> The test is skipped with return code 0.
> 
> Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
> ---
>  tools/perf/tests/attr.py | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/tools/perf/tests/attr.py b/tools/perf/tests/attr.py
> index d0242d8..6e71d52 100644
> --- a/tools/perf/tests/attr.py
> +++ b/tools/perf/tests/attr.py
> @@ -30,6 +30,13 @@ class Fail(Exception):
>      def getMsg(self):
>          return '\'%s\' - %s' % (self.test.path, self.msg)
>  
> +class Notest(Exception):
> +    def __init__(self, test, arch):
> +        self.arch = arch
> +        self.test = test
> +    def getMsg(self):
> +        return '[%s] \'%s\'' % (self.arch, self.test.path)
> +
>  class Unsup(Exception):
>      def __init__(self, test):
>          self.test = test
> @@ -112,6 +119,7 @@ class Event(dict):
>  #     'command' - perf command name
>  #     'args'    - special command arguments
>  #     'ret'     - expected command return value (0 by default)
> +#     'machine' - machine specific test
>  #
>  # [eventX:base]
>  #   - one or multiple instances in file
> @@ -134,6 +142,12 @@ class Test(object):
>          except:
>              self.ret  = 0
>  
> +        try:
> +            self.machine  = parser.get('config', 'machine')
> +            log.warning("test limitation '%s'" % self.machine)
> +        except:
> +            self.machine  = ''
> +
>          self.expect   = {}
>          self.result   = {}
>          log.debug("  loading expected events");
> @@ -145,6 +159,30 @@ class Test(object):
>          else:
>              return True
>  
> +    def skip_test(self, arch):
> +        # If architecture not set always run test
> +        if self.machine == '':
> +            return False
> +
> +        # Allow multiple values in assignment separated by ','
> +        machine_list = self.machine.split(',')
> +
> +        # Handle negated list such as !s390x,ppc
> +        if machine_list[0][0] == '!':
> +            machine_list[0] = machine_list[0][1:]
> +            log.warning("excluded machine_list %s" % machine_list)
> +            for machine_item in machine_list:
> +                # log.warning("test for %s arch is %s" % (machine_item, arch))
> +                if machine_item == arch:
> +                    return True
> +            return False
> +
> +        for machine_item in machine_list:
> +            # log.warning("test for %s arch is %s" % (machine_item, arch))
> +            if machine_item == arch:
> +                return False
> +        return True
> +
>      def load_events(self, path, events):
>          parser_event = ConfigParser.SafeConfigParser()
>          parser_event.read(path)
> @@ -168,6 +206,11 @@ class Test(object):
>              events[section] = e
>  
>      def run_cmd(self, tempdir):
> +        junk1, junk2, junk3, junk4, arch = (os.uname())
> +
> +        if self.skip_test(arch):
> +            raise Notest(self, arch)
> +
>          cmd = "PERF_TEST_ATTR=%s %s %s -o %s/perf.data %s" % (tempdir,
>                self.perf, self.command, tempdir, self.args)
>          ret = os.WEXITSTATUS(os.system(cmd))
> @@ -267,6 +310,8 @@ def run_tests(options):
>              Test(f, options).run()
>          except Unsup, obj:
>              log.warning("unsupp  %s" % obj.getMsg())
> +        except Notest, obj:
> +            log.warning("skipped %s" % obj.getMsg())
>  
>  def setup_log(verbose):
>      global log
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RFC perf test 14 add platform dependency
  2017-06-16 16:08 ` Arnaldo Carvalho de Melo
@ 2017-06-18 22:51   ` Jiri Olsa
  0 siblings, 0 replies; 9+ messages in thread
From: Jiri Olsa @ 2017-06-18 22:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Thomas Richter, linux-perf-users

On Fri, Jun 16, 2017 at 01:08:17PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jun 13, 2017 at 11:09:37AM +0200, Thomas Richter escreveu:
> > This is a proposal to add platform dependency into the
> > test case 14 (perf_event_attr). It is based on a suggestion from
> > Jiri Olsa.
> > Add a new optional attribute named 'machine' in the [config] section
> > of the test case file. It is a comma separated list of architecture
> > names this test can be executed on. For example:
> > 
> > machine = x86_64,alpha,ppc
> 
> Looks ok, Jiri?

sorry on late response, I'll look on it shortly

jirka

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RFC perf test 14 add platform dependency
  2017-06-13  9:09 [PATCH v2] RFC perf test 14 add platform dependency Thomas Richter
  2017-06-16 16:08 ` Arnaldo Carvalho de Melo
@ 2017-06-19 20:59 ` Jiri Olsa
  2017-06-20  1:40   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2017-06-19 20:59 UTC (permalink / raw)
  To: Thomas Richter; +Cc: linux-perf-users

On Tue, Jun 13, 2017 at 11:09:37AM +0200, Thomas Richter wrote:
> This is a proposal to add platform dependency into the
> test case 14 (perf_event_attr). It is based on a suggestion from
> Jiri Olsa.
> Add a new optional attribute named 'machine' in the [config] section
> of the test case file. It is a comma separated list of architecture
> names this test can be executed on. For example:
> 
> machine = x86_64,alpha,ppc

'arch' sounds better to me, but machine is ok I guess

> 
> If this attribute is missing the test is executed on any platform.
> This does not break the current setup.
> The values listed for this attribute should be identical to
> uname -m output.
> If the list starts with an exclamation mark (!) the comparison is
> inverted, for example for
> 
> machine = !s390x,ppc
> 
> the test is not executed on s390x or ppc platforms.
> The exclamation mark must be at the beginnning of the list.

could that be per arch? this made me think that it's not s390 and it IS for ppc

> 
> Here is an example debug output:
> [root@s35lp76]# fgrep machine tests/attr/test-stat-C2
> machine = x86_64,alpha,ppc
> [root@s35lp76]# PERF_TEST_ATTR=/tmp /usr/bin/python2 ./tests/attr.py \
>   -d ./tests/attr/ -p ./perf -vvvvv -t test-stat-C1
> 
> provides the following output:
> 
> running './tests/attr//test-stat-C1'
> test executed only on 'x86_64,alpha,ppc' <--- new
>   loading expected events
>     Event event:base-stat
>       fd = 1
>       group_fd = -1
>       .....
> skipped [s390x] './tests/attr//test-stat-C1' <--- new

is this mixed output from supported and non supported archs?
if the arch is skip there's no output other than the skip line right?


ther than above nits it looks ok to me

thanks,
jirka

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RFC perf test 14 add platform dependency
  2017-06-19 20:59 ` Jiri Olsa
@ 2017-06-20  1:40   ` Arnaldo Carvalho de Melo
  2017-06-20  8:12     ` Thomas-Mich Richter
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-06-20  1:40 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Thomas Richter, linux-perf-users

Em Mon, Jun 19, 2017 at 10:59:44PM +0200, Jiri Olsa escreveu:
> On Tue, Jun 13, 2017 at 11:09:37AM +0200, Thomas Richter wrote:
> > This is a proposal to add platform dependency into the
> > test case 14 (perf_event_attr). It is based on a suggestion from
> > Jiri Olsa.
> > Add a new optional attribute named 'machine' in the [config] section
> > of the test case file. It is a comma separated list of architecture
> > names this test can be executed on. For example:
> > 
> > machine = x86_64,alpha,ppc
> 
> 'arch' sounds better to me, but machine is ok I guess

Agreed, for consistency with other places, like in struct machine, that
represents virtual and host 'machines' that have an 'arch', etc.

So better to rename "machine" above to "arch".
 
> > If this attribute is missing the test is executed on any platform.
> > This does not break the current setup.
> > The values listed for this attribute should be identical to
> > uname -m output.
> > If the list starts with an exclamation mark (!) the comparison is
> > inverted, for example for
> > 
> > machine = !s390x,ppc
> > 
> > the test is not executed on s390x or ppc platforms.
> > The exclamation mark must be at the beginnning of the list.
> 
> could that be per arch? this made me think that it's not s390 and it IS for ppc

yeah, having the ! affect just the arch right after it looks more
flexible and clear.
 
> > Here is an example debug output:
> > [root@s35lp76]# fgrep machine tests/attr/test-stat-C2
> > machine = x86_64,alpha,ppc
> > [root@s35lp76]# PERF_TEST_ATTR=/tmp /usr/bin/python2 ./tests/attr.py \
> >   -d ./tests/attr/ -p ./perf -vvvvv -t test-stat-C1
> > 
> > provides the following output:
> > 
> > running './tests/attr//test-stat-C1'
> > test executed only on 'x86_64,alpha,ppc' <--- new
> >   loading expected events
> >     Event event:base-stat
> >       fd = 1
> >       group_fd = -1
> >       .....
> > skipped [s390x] './tests/attr//test-stat-C1' <--- new
> 
> is this mixed output from supported and non supported archs?
> if the arch is skip there's no output other than the skip line right?
> 
> 
> ther than above nits it looks ok to me
> 
> thanks,
> jirka
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RFC perf test 14 add platform dependency
  2017-06-20  1:40   ` Arnaldo Carvalho de Melo
@ 2017-06-20  8:12     ` Thomas-Mich Richter
  2017-06-20  9:24       ` Jiri Olsa
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas-Mich Richter @ 2017-06-20  8:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa; +Cc: linux-perf-users, Hendrik Brueckner

On 06/20/2017 03:40 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 19, 2017 at 10:59:44PM +0200, Jiri Olsa escreveu:
>> On Tue, Jun 13, 2017 at 11:09:37AM +0200, Thomas Richter wrote:
>>> This is a proposal to add platform dependency into the
>>> test case 14 (perf_event_attr). It is based on a suggestion from
>>> Jiri Olsa.
>>> Add a new optional attribute named 'machine' in the [config] section
>>> of the test case file. It is a comma separated list of architecture
>>> names this test can be executed on. For example:
>>>
>>> machine = x86_64,alpha,ppc
>>
>> 'arch' sounds better to me, but machine is ok I guess
> 
> Agreed, for consistency with other places, like in struct machine, that
> represents virtual and host 'machines' that have an 'arch', etc.
> 
> So better to rename "machine" above to "arch".
> 

Ok lets rename it to arch.

>>> If this attribute is missing the test is executed on any platform.
>>> This does not break the current setup.
>>> The values listed for this attribute should be identical to
>>> uname -m output.
>>> If the list starts with an exclamation mark (!) the comparison is
>>> inverted, for example for
>>>
>>> machine = !s390x,ppc
>>>
>>> the test is not executed on s390x or ppc platforms.
>>> The exclamation mark must be at the beginnning of the list.
>>
>> could that be per arch? this made me think that it's not s390 and it IS for ppc
> 
> yeah, having the ! affect just the arch right after it looks more
> flexible and clear.
> 

Does this really have any benefit? Assume
arch = !s390x,ppc

I will scan the list from left to right and the first match wins.
Checking the first word in the list determines
this test is executed on anything but s390x. So the 2nd word in the
list (ppc) is useless. Specifying more architectures makes only sense
when you negate them such as
arch = !s390x,!ppc

That is the reason why I ended up with the original proposal.
Any suggestions?

-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RFC perf test 14 add platform dependency
  2017-06-20  8:12     ` Thomas-Mich Richter
@ 2017-06-20  9:24       ` Jiri Olsa
  2017-06-20 14:06         ` Thomas-Mich Richter
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Olsa @ 2017-06-20  9:24 UTC (permalink / raw)
  To: Thomas-Mich Richter
  Cc: Arnaldo Carvalho de Melo, linux-perf-users, Hendrik Brueckner

On Tue, Jun 20, 2017 at 10:12:14AM +0200, Thomas-Mich Richter wrote:
> On 06/20/2017 03:40 AM, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jun 19, 2017 at 10:59:44PM +0200, Jiri Olsa escreveu:
> >> On Tue, Jun 13, 2017 at 11:09:37AM +0200, Thomas Richter wrote:
> >>> This is a proposal to add platform dependency into the
> >>> test case 14 (perf_event_attr). It is based on a suggestion from
> >>> Jiri Olsa.
> >>> Add a new optional attribute named 'machine' in the [config] section
> >>> of the test case file. It is a comma separated list of architecture
> >>> names this test can be executed on. For example:
> >>>
> >>> machine = x86_64,alpha,ppc
> >>
> >> 'arch' sounds better to me, but machine is ok I guess
> > 
> > Agreed, for consistency with other places, like in struct machine, that
> > represents virtual and host 'machines' that have an 'arch', etc.
> > 
> > So better to rename "machine" above to "arch".
> > 
> 
> Ok lets rename it to arch.
> 
> >>> If this attribute is missing the test is executed on any platform.
> >>> This does not break the current setup.
> >>> The values listed for this attribute should be identical to
> >>> uname -m output.
> >>> If the list starts with an exclamation mark (!) the comparison is
> >>> inverted, for example for
> >>>
> >>> machine = !s390x,ppc
> >>>
> >>> the test is not executed on s390x or ppc platforms.
> >>> The exclamation mark must be at the beginnning of the list.
> >>
> >> could that be per arch? this made me think that it's not s390 and it IS for ppc
> > 
> > yeah, having the ! affect just the arch right after it looks more
> > flexible and clear.
> > 
> 
> Does this really have any benefit? Assume
> arch = !s390x,ppc
> 
> I will scan the list from left to right and the first match wins.
> Checking the first word in the list determines
> this test is executed on anything but s390x. So the 2nd word in the
> list (ppc) is useless. Specifying more architectures makes only sense
> when you negate them such as
> arch = !s390x,!ppc

ok, makes sense

thanks,
jirka

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RFC perf test 14 add platform dependency
  2017-06-20  9:24       ` Jiri Olsa
@ 2017-06-20 14:06         ` Thomas-Mich Richter
  2017-06-20 14:48           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas-Mich Richter @ 2017-06-20 14:06 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Arnaldo Carvalho de Melo, linux-perf-users, Hendrik Brueckner

On 06/20/2017 11:24 AM, Jiri Olsa wrote:
> On Tue, Jun 20, 2017 at 10:12:14AM +0200, Thomas-Mich Richter wrote:
>> On 06/20/2017 03:40 AM, Arnaldo Carvalho de Melo wrote:
>>> Em Mon, Jun 19, 2017 at 10:59:44PM +0200, Jiri Olsa escreveu:
>>>> On Tue, Jun 13, 2017 at 11:09:37AM +0200, Thomas Richter wrote:
>>>>> This is a proposal to add platform dependency into the
>>>>> test case 14 (perf_event_attr). It is based on a suggestion from
>>>>> Jiri Olsa.
>>>>> Add a new optional attribute named 'machine' in the [config] section
>>>>> of the test case file. It is a comma separated list of architecture
>>>>> names this test can be executed on. For example:
>>>>>
>>>>> machine = x86_64,alpha,ppc
>>>>
>>>> 'arch' sounds better to me, but machine is ok I guess
>>>
>>> Agreed, for consistency with other places, like in struct machine, that
>>> represents virtual and host 'machines' that have an 'arch', etc.
>>>
>>> So better to rename "machine" above to "arch".
>>>
>>
>> Ok lets rename it to arch.
>>
>>>>> If this attribute is missing the test is executed on any platform.
>>>>> This does not break the current setup.
>>>>> The values listed for this attribute should be identical to
>>>>> uname -m output.
>>>>> If the list starts with an exclamation mark (!) the comparison is
>>>>> inverted, for example for
>>>>>
>>>>> machine = !s390x,ppc
>>>>>
>>>>> the test is not executed on s390x or ppc platforms.
>>>>> The exclamation mark must be at the beginnning of the list.
>>>>
>>>> could that be per arch? this made me think that it's not s390 and it IS for ppc
>>>
>>> yeah, having the ! affect just the arch right after it looks more
>>> flexible and clear.
>>>
>>
>> Does this really have any benefit? Assume
>> arch = !s390x,ppc
>>
>> I will scan the list from left to right and the first match wins.
>> Checking the first word in the list determines
>> this test is executed on anything but s390x. So the 2nd word in the
>> list (ppc) is useless. Specifying more architectures makes only sense
>> when you negate them such as
>> arch = !s390x,!ppc
> 
> ok, makes sense
> 
> thanks,
> jirka
> 

I will rewrite the patch for Linux 4.12 as soon as Arnaldo agrees 
with us.

-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] RFC perf test 14 add platform dependency
  2017-06-20 14:06         ` Thomas-Mich Richter
@ 2017-06-20 14:48           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-06-20 14:48 UTC (permalink / raw)
  To: Thomas-Mich Richter; +Cc: Jiri Olsa, linux-perf-users, Hendrik Brueckner

Em Tue, Jun 20, 2017 at 04:06:15PM +0200, Thomas-Mich Richter escreveu:
> On 06/20/2017 11:24 AM, Jiri Olsa wrote:
> > On Tue, Jun 20, 2017 at 10:12:14AM +0200, Thomas-Mich Richter wrote:
> >> On 06/20/2017 03:40 AM, Arnaldo Carvalho de Melo wrote:
> >>> Em Mon, Jun 19, 2017 at 10:59:44PM +0200, Jiri Olsa escreveu:
> >>>> On Tue, Jun 13, 2017 at 11:09:37AM +0200, Thomas Richter wrote:
> >>>>> This is a proposal to add platform dependency into the
> >>>>> test case 14 (perf_event_attr). It is based on a suggestion from
> >>>>> Jiri Olsa.
> >>>>> Add a new optional attribute named 'machine' in the [config] section
> >>>>> of the test case file. It is a comma separated list of architecture
> >>>>> names this test can be executed on. For example:
> >>>>>
> >>>>> machine = x86_64,alpha,ppc
> >>>>
> >>>> 'arch' sounds better to me, but machine is ok I guess
> >>>
> >>> Agreed, for consistency with other places, like in struct machine, that
> >>> represents virtual and host 'machines' that have an 'arch', etc.
> >>>
> >>> So better to rename "machine" above to "arch".
> >>>
> >>
> >> Ok lets rename it to arch.
> >>
> >>>>> If this attribute is missing the test is executed on any platform.
> >>>>> This does not break the current setup.
> >>>>> The values listed for this attribute should be identical to
> >>>>> uname -m output.
> >>>>> If the list starts with an exclamation mark (!) the comparison is
> >>>>> inverted, for example for
> >>>>>
> >>>>> machine = !s390x,ppc
> >>>>>
> >>>>> the test is not executed on s390x or ppc platforms.
> >>>>> The exclamation mark must be at the beginnning of the list.
> >>>>
> >>>> could that be per arch? this made me think that it's not s390 and it IS for ppc
> >>>
> >>> yeah, having the ! affect just the arch right after it looks more
> >>> flexible and clear.
> >>>
> >>
> >> Does this really have any benefit? Assume
> >> arch = !s390x,ppc
> >>
> >> I will scan the list from left to right and the first match wins.
> >> Checking the first word in the list determines
> >> this test is executed on anything but s390x. So the 2nd word in the
> >> list (ppc) is useless. Specifying more architectures makes only sense
> >> when you negate them such as
> >> arch = !s390x,!ppc
> > 
> > ok, makes sense
> > 
> > thanks,
> > jirka
> > 
> 
> I will rewrite the patch for Linux 4.12 as soon as Arnaldo agrees 
> with us.

Yeah, that was me wanting this to have the same syntax as 'perf script
-F', where we start with a default list of fields and it makes sense to
ask for fields to be removed and, at the same time, for some other
fields to be added.

Here this is not the case, we start with: all arches, then we want to
remove some, so just having it at the start, as you did, is the right
thing, duh.

Sorry for the noise...

- Arnaldo

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-06-20 14:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-13  9:09 [PATCH v2] RFC perf test 14 add platform dependency Thomas Richter
2017-06-16 16:08 ` Arnaldo Carvalho de Melo
2017-06-18 22:51   ` Jiri Olsa
2017-06-19 20:59 ` Jiri Olsa
2017-06-20  1:40   ` Arnaldo Carvalho de Melo
2017-06-20  8:12     ` Thomas-Mich Richter
2017-06-20  9:24       ` Jiri Olsa
2017-06-20 14:06         ` Thomas-Mich Richter
2017-06-20 14:48           ` Arnaldo Carvalho de Melo

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 NNTP newsgroup(s).