Andreas Krebbel | 2 Nov 2011 16:10
Picon

[PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

This patch makes use of the event mechanism to allow for dynamic
enabling and disabling of the System z hardware sampling facility with
the OProfile user space tools.

A single virtual counter is created which can be used to
enable/disable hardware sampling dynamically from user space.  The
counter can be used with the two events 0 and 1.  Using event 0
enables use of timer based sampling while event 1 turns on hardware
sampling.  These values have to stay like this since the
/dev/oprofile/0/event always has to match
/dev/oprofile/hwsampling/hwsampler content in order to support the
existing interface in parallel.  Apart from 'event' only the 'kernel'
and 'user' flags are evaluated by the kernel code.

This adds two new opcontrol options --s390hwsampbufsize and
--s390hwsamprate which can be used to set the two hardware sampling
specific values at opcontrol cmdline.

Signed-off-by: Andreas Krebbel <krebbel <at> linux.vnet.ibm.com>
---
 doc/oprofile.xml                               |   62 ++++++++++++++++++
 events/Makefile.am                             |    4 -
 events/s390x/basic_mode_sampling_v1/events     |    8 ++
 events/s390x/basic_mode_sampling_v1/unit_masks |    8 ++
 libop/op_cpu_type.c                            |    1 
 libop/op_cpu_type.h                            |    1 
 libop/op_events.c                              |    3 
 libpp/op_header.cpp                            |   10 +-
 utils/opcontrol                                |   84 +++++++++++++++++++++++++
 utils/ophelp.c                                 |    4 +
(Continue reading)

Maynard Johnson | 2 Nov 2011 17:54
Picon
Favicon

Re: [PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

Andreas Krebbel wrote:
> This patch makes use of the event mechanism to allow for dynamic
> enabling and disabling of the System z hardware sampling facility with
> the OProfile user space tools.

Andreas,
This patch does not apply cleanly to current upstream code.  Please generate a patch against current code.

The patch is definitely a lot closer to being ready to accept.  Just a few minor issues.  See below.

-Maynard

> 
> A single virtual counter is created which can be used to
> enable/disable hardware sampling dynamically from user space.  The
> counter can be used with the two events 0 and 1.  Using event 0
> enables use of timer based sampling while event 1 turns on hardware
> sampling.  These values have to stay like this since the
> /dev/oprofile/0/event always has to match
> /dev/oprofile/hwsampling/hwsampler content in order to support the
> existing interface in parallel.  Apart from 'event' only the 'kernel'
> and 'user' flags are evaluated by the kernel code.
> 
> This adds two new opcontrol options --s390hwsampbufsize and
> --s390hwsamprate which can be used to set the two hardware sampling
> specific values at opcontrol cmdline.

As we discussed in our IRC chat, we won't need the new 's390hwsamprate' option, since the "count" value in
the event spec should cover this.

(Continue reading)

Maynard Johnson | 2 Nov 2011 18:05
Picon
Favicon

Re: [PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

Andreas Krebbel wrote:
> This patch makes use of the event mechanism to allow for dynamic
> enabling and disabling of the System z hardware sampling facility with
> the OProfile user space tools.
[snip]
> +# Various checks related to Hardware sampler option
> +check_platform_args()
> +{
> +	case "$CPUTYPE" in
> +		s390x/basic_mode_sampling_v1)
> +			if test "$S390_HW_SAMPLER_RATE" == "0"; then
> +				return
> +			fi
> +			if test -r $MOUNT/hwsampling/hw_min_interval; then
> +				min_interval=`cat $MOUNT/hwsampling/hw_min_interval`
> +			fi
> +			if test -r $MOUNT/hwsampling/hw_max_interval; then
> +				max_interval=`cat $MOUNT/hwsampling/hw_max_interval`
> + 			fi
> +

I forgot to mention this in my previous response to your patch . . . but if either of the above tests fail and you
don't get min_interval or max_interval set, then the below test makes no sense and will fail.

-Maynard

> +			if test "$S390_HW_SAMPLER_RATE" -lt "$min_interval" -o \
> +				"$S390_HW_SAMPLER_RATE" -gt "$max_interval"; then
> +				echo "Invalid value for hardware sampling rate" >&2
> +				echo "should be between $min_interval and $max_interval" >&2
(Continue reading)

Andreas Krebbel | 3 Nov 2011 12:56
Picon

[PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

This patch makes use of the event mechanism to allow for dynamic
enabling and disabling of the System z hardware sampling facility with
the OProfile user space tools.

A single virtual counter is created which can be used to
enable/disable hardware sampling dynamically from user space.  The
counter can be used with the two events 0 and 1.  Using event 0
enables use of timer based sampling while event 1 turns on hardware
sampling.  These values have to stay like this since the
/dev/oprofile/0/event always has to match
/dev/oprofile/hwsampling/hwsampler content in order to support the
existing interface in parallel. 

This adds the new opcontrol option --s390hwsampbufsize.  This option
specifies the number of 2MB memory areas allocated per CPU by the
kernel module.

Signed-off-by: Andreas Krebbel <krebbel <at> linux.vnet.ibm.com>
---
 doc/opcontrol.1.in                             |   10 ++++
 doc/oprofile.xml                               |   58 +++++++++++++++++++++++
 events/Makefile.am                             |    4 +-
 events/s390x/basic_mode_sampling_v1/events     |    8 +++
 events/s390x/basic_mode_sampling_v1/unit_masks |    8 +++
 libop/op_cpu_type.c                            |    1 +
 libop/op_cpu_type.h                            |    1 +
 libop/op_events.c                              |    3 +
 libpp/op_header.cpp                            |   10 ++--
 utils/opcontrol                                |   59 ++++++++++++++++++++++++
 utils/ophelp.c                                 |    6 ++
(Continue reading)

Maynard Johnson | 3 Nov 2011 16:21
Picon
Favicon

Re: [PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

On 11/03/2011 6:56 AM, Andreas Krebbel wrote:
> This patch makes use of the event mechanism to allow for dynamic
> enabling and disabling of the System z hardware sampling facility with
> the OProfile user space tools.

Andreas,
Thanks for the patch.  Other than a few minor things (which I can fix when I 
commit), I have just one question below concerning S390_HW_SAMPLER_BUFSIZE.

-Maynard

>
> A single virtual counter is created which can be used to
> enable/disable hardware sampling dynamically from user space.  The
> counter can be used with the two events 0 and 1.  Using event 0
> enables use of timer based sampling while event 1 turns on hardware
> sampling.  These values have to stay like this since the
> /dev/oprofile/0/event always has to match
> /dev/oprofile/hwsampling/hwsampler content in order to support the
> existing interface in parallel.
>
> This adds the new opcontrol option --s390hwsampbufsize.  This option
> specifies the number of 2MB memory areas allocated per CPU by the
> kernel module.
>
> Signed-off-by: Andreas Krebbel<krebbel <at> linux.vnet.ibm.com>
> ---
>   doc/opcontrol.1.in                             |   10 ++++
>   doc/oprofile.xml                               |   58 +++++++++++++++++++++++
>   events/Makefile.am                             |    4 +-
(Continue reading)

Andreas Krebbel | 3 Nov 2011 16:44
Picon

Re: [PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

On 11/03/2011 04:21 PM, Maynard Johnson wrote:
> On 11/03/2011 6:56 AM, Andreas Krebbel wrote:
>> This patch makes use of the event mechanism to allow for dynamic
>> enabling and disabling of the System z hardware sampling facility with
>> the OProfile user space tools.
> 
> Andreas,
> Thanks for the patch.  Other than a few minor things (which I can fix when I 
> commit), I have just one question below concerning S390_HW_SAMPLER_BUFSIZE.

>> +	if test "$S390_HW_SAMPLER" = "1"; then
>> +		echo -n "System z hardware sampling buffer size (in 2MB areas): "
> 
> Using two echos to get the description and value displayed will end up printing 
> two lines.  We don't want that.

The -n suppresses a new line to be added.

>> +		if test "$S390_HW_SAMPLER_BUFSIZE" = "0"; then
>> +			cat $MOUNT/hwsampling/hw_sdbt_blocks
> 
> I don't think this if/else is appropriate.  The S390_HW_SAMPLER_BUFSIZE is 
> cached in daemonrc, and at this point in opcontrol, we will have updated the 
> S390_HW_SAMPLER_BUFSIZE variable with the cached value.  So just always use 
> S390_HW_SAMPLER_BUFSIZE.  That's what we do for all other profiling values.  Or 
> am I missing something?

The hardware sampling module itself uses a sane default if not configured otherwise.  The
value of S390_HW_SAMPLER_BUFSIZE is only written to the setup file if it is configured to
!=0 otherwise the kernel module default is being used. So the user in that case probably
(Continue reading)

Maynard Johnson | 3 Nov 2011 17:38
Picon
Favicon

Re: [PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

On 11/03/2011 10:44 AM, Andreas Krebbel wrote:
> On 11/03/2011 04:21 PM, Maynard Johnson wrote:
>> On 11/03/2011 6:56 AM, Andreas Krebbel wrote:
>>> This patch makes use of the event mechanism to allow for dynamic
>>> enabling and disabling of the System z hardware sampling facility with
>>> the OProfile user space tools.

Patch committed with minor fixups as mentioned in my earlier reply.  Please do a 
fresh checkout from upstream git repo and verify everything is OK.

-Maynard

>>
>> Andreas,
>> Thanks for the patch.  Other than a few minor things (which I can fix when I
>> commit), I have just one question below concerning S390_HW_SAMPLER_BUFSIZE.
>
>>> +	if test "$S390_HW_SAMPLER" = "1"; then
>>> +		echo -n "System z hardware sampling buffer size (in 2MB areas): "
>>
>> Using two echos to get the description and value displayed will end up printing
>> two lines.  We don't want that.
>
> The -n suppresses a new line to be added.
>
>>> +		if test "$S390_HW_SAMPLER_BUFSIZE" = "0"; then
>>> +			cat $MOUNT/hwsampling/hw_sdbt_blocks
>>
>> I don't think this if/else is appropriate.  The S390_HW_SAMPLER_BUFSIZE is
>> cached in daemonrc, and at this point in opcontrol, we will have updated the
(Continue reading)

Andreas Krebbel | 4 Nov 2011 08:53
Picon

Re: [PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

On 11/03/2011 05:38 PM, Maynard Johnson wrote:
> On 11/03/2011 10:44 AM, Andreas Krebbel wrote:
>> On 11/03/2011 04:21 PM, Maynard Johnson wrote:
>>> On 11/03/2011 6:56 AM, Andreas Krebbel wrote:
>>>> This patch makes use of the event mechanism to allow for dynamic
>>>> enabling and disabling of the System z hardware sampling facility with
>>>> the OProfile user space tools.
> 
> Patch committed with minor fixups as mentioned in my earlier reply.  Please do a 
> fresh checkout from upstream git repo and verify everything is OK.

Thanks!
I did a few tests and it seems to work fine. However, could you please merge the patch
below. It fixes a typo in one of the variables:

>> I found a typo while looking at it. Could you please fix that one as well?
>> --- a/utils/opcontrol
>> +++ b/utils/opcontrol
>>  <at>  <at>  -507,7 +507,7  <at>  <at>  do_save_setup()
>>          if test "$XEN_RANGE"; then
>>                  echo "XEN_RANGE=$XEN_RANGE">>  $SETUP_FILE
>>          fi
>> -       if test "$S390_HW_SAMPLER" = "1" -a "$S390_HWSAMPLER_BUFSIZE" != "0"; then
>> +       if test "$S390_HW_SAMPLER" = "1" -a "$S390_HW_SAMPLER_BUFSIZE" != "0"; then
>>                  echo "S390_HW_SAMPLER_BUFSIZE=$S390_HW_SAMPLER_BUFSIZE">>  $SETUP_FILE
>>          fi
>>          SETUP_FILE="$SAVE_SETUP_FILE"
>>

Bye,
(Continue reading)

Maynard Johnson | 4 Nov 2011 14:37
Picon
Favicon

Re: [PATCH 2/2] S/390: Enhance the user space tools for System z hardware sampling

Andreas Krebbel wrote:
> On 11/03/2011 05:38 PM, Maynard Johnson wrote:
>> On 11/03/2011 10:44 AM, Andreas Krebbel wrote:
>>> On 11/03/2011 04:21 PM, Maynard Johnson wrote:
>>>> On 11/03/2011 6:56 AM, Andreas Krebbel wrote:
>>>>> This patch makes use of the event mechanism to allow for dynamic
>>>>> enabling and disabling of the System z hardware sampling facility with
>>>>> the OProfile user space tools.
>>
>> Patch committed with minor fixups as mentioned in my earlier reply.  Please do a 
>> fresh checkout from upstream git repo and verify everything is OK.
> 
> Thanks!
> I did a few tests and it seems to work fine. However, could you please merge the patch
> below. It fixes a typo in one of the variables:
Done.

-Maynard
> 
>>> I found a typo while looking at it. Could you please fix that one as well?
>>> --- a/utils/opcontrol
>>> +++ b/utils/opcontrol
>>>  <at>  <at>  -507,7 +507,7  <at>  <at>  do_save_setup()
>>>          if test "$XEN_RANGE"; then
>>>                  echo "XEN_RANGE=$XEN_RANGE">>  $SETUP_FILE
>>>          fi
>>> -       if test "$S390_HW_SAMPLER" = "1" -a "$S390_HWSAMPLER_BUFSIZE" != "0"; then
>>> +       if test "$S390_HW_SAMPLER" = "1" -a "$S390_HW_SAMPLER_BUFSIZE" != "0"; then
>>>                  echo "S390_HW_SAMPLER_BUFSIZE=$S390_HW_SAMPLER_BUFSIZE">>  $SETUP_FILE
>>>          fi
(Continue reading)


Gmane