Skip to contentSkip to conversation

OProfile support for System z's hardware sampling

PATCH Superseded [email protected] 2 participants 13 messages Dec 20, 2010 – Jan 19, 2011 s390

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover OProfile support for System z's hardware sampling

So far, OProfile takes samples by using a software interrupt.
The purpose of this series of patches is to add support for System z hardware sampling to OProfile.

Hardware (HW) sampling is a feature provided by System z processors (z10 and follow ons). When sampling, the processor takes samples containing the instruction address, PID, and other information. The samples are taken at a programmable rate and stored into a buffer provided by the operating system. The sampling process is implemented in hardware and millicode and thus does not affect the operating system being oberved, apart from requiring buffer memory that the Linux kernel must provide.

Hardware sampling is available in LPAR mode on 64 BIT processors only.

The overall approach is to replace the software-based sample generation by hardware sampling. The driver for the HW sampling mechanism is a kernel module named hwsampler. Kernel module hwsampler provides functions for
- controlling the sampling hardware,
- setting up appropriate buffer structures (HW buffers),
- retrieving sample entries from these buffers.
Kernel module hwsampler can handle multiple CPUs.

The samples contain the instruction address, a bit distinguishing between kernel and user space, and for user space samples also the PID.
Instead of taking samples from its own per-CPU buffers, OProfile would rather take samples from the HW buffers.

When hardware sampling can be enabled on the current System z processor it will be the new default. Switching back to timer based sampling can be established by using

echo 0 > /dev/oprofile/hwsampling/hwsampler

The user space drivers of OProfile also need an extension to control hw sampling by appropriate options.

#1 lore raw

PATCH 1/4 This patch adds support for hardware based sampling on System z processors (models z10 and up)

From: [email protected]

The CPU Measurement Facility CPUMF is described in the z/Architecture Principles of Operation.

The patch introduces
- a new configuration option OPROFILE_HWSAMPLING_MODE
- a new kernel module hwsampler that controls all hardware sampling related operations as
- checking if hardware sampling feature is available
- ie: on System z models z10 and up, in LPAR mode only, and authorised during LPAR activation
- allocating memory for the hardware sampling feature
- starting/stopping hardware sampling
The hwsampler module will also depend on CONFIG_OPROFILE and CONFIG_64BIT.

All functions required to start and stop hardware sampling have to be invoked by the oprofile kernel module as provided by the other patches of this patch set.

In case hardware based sampling cannot be setup standard timer based sampling is used by OProfile.

---
 arch/s390/Kconfig                       |   22 
 arch/s390/include/asm/hwsampler.h       |  130 +++
 arch/s390/oprofile/Makefile             |    6 
 drivers/s390/hwsampler/hwsampler-main.c | 1155 ++++++++++++++++++++++++++++++++
 drivers/s390/hwsampler/smpctl.c         |  170 ++++
 5 files changed, 1483 insertions(+)
arch/s390/include/asm/hwsampler.h new file +130 -0
Index: linux-2.6/arch/s390/include/asm/hwsampler.h
===================================================================
@@ -0,0 +1,130 @@
+/*
+ * CPUMF HW sampler structures and prototypes
+ *
+ *    Copyright IBM Corp. 2010
+ *    Author(s): Heinz Graalfs <[email protected]>
+ */
+
+#ifndef HWSAMPLER_H_
+#define HWSAMPLER_H_
+
+#include <linux/workqueue.h>
+
+struct qsi_info_block          /* QUERY SAMPLING information block  */
+{ /* Bit(s) */
+	unsigned int b0_13:14;      /* 0-13: zeros                       */
+	unsigned int as:1;          /* 14: sampling authorisation control*/
+	unsigned int b15_21:7;      /* 15-21: zeros                      */
+	unsigned int es:1;          /* 22: sampling enable control       */
+	unsigned int b23_29:7;      /* 23-29: zeros                      */
+	unsigned int cs:1;          /* 30: sampling activation control   */
+	unsigned int:1;             /* 31: reserved                      */
+	unsigned int bsdes:16;      /* 4-5: size of sampling entry       */
+	unsigned int:16;            /* 6-7: reserved                     */
+	unsigned long min_sampl_rate; /* 8-15: minimum sampling interval */
+	unsigned long max_sampl_rate; /* 16-23: maximum sampling interval*/
+	unsigned long tear;         /* 24-31: TEAR contents              */
+	unsigned long dear;         /* 32-39: DEAR contents              */
+	unsigned int rsvrd0;        /* 40-43: reserved                   */
+	unsigned int cpu_speed;     /* 44-47: CPU speed                  */
+	unsigned long long rsvrd1;  /* 48-55: reserved                   */
+	unsigned long long rsvrd2;  /* 56-63: reserved                   */
+};
+
+struct ssctl_request_block     /* SET SAMPLING CONTROLS req block   */
+{ /* bytes 0 - 7  Bit(s) */
+	unsigned int s:1;           /* 0: maximum buffer indicator       */
+	unsigned int h:1;           /* 1: part. level reserved for VM use*/
+	unsigned long b2_53:52;     /* 2-53: zeros                       */
+	unsigned int es:1;          /* 54: sampling enable control       */
+	unsigned int b55_61:7;      /* 55-61: - zeros                    */
+	unsigned int cs:1;          /* 62: sampling activation control   */
+	unsigned int b63:1;         /* 63: zero                          */
+	unsigned long interval;     /* 8-15: sampling interval           */
+	unsigned long tear;         /* 16-23: TEAR contents              */
+	unsigned long dear;         /* 24-31: DEAR contents              */
+	/* 32-63:                                                        */
+	unsigned long rsvrd1;       /* reserved                          */
+	unsigned long rsvrd2;       /* reserved                          */
+	unsigned long rsvrd3;       /* reserved                          */
+	unsigned long rsvrd4;       /* reserved                          */
+};
+
+typedef void oprf_add_sample_func(unsigned long pc,
+	struct pt_regs * const regs,
+	unsigned long event, int is_kernel,
+	struct task_struct *task);
+
+struct cpu_buffer {
+	unsigned long first_sdbt;       /* @ of 1st SDB-Table for this CP*/
+	unsigned long worker_entry;
+	unsigned long sample_overflow;  /* taken from SDB ...            */
+	struct qsi_info_block qsi;
+	struct ssctl_request_block ssctl;
+	struct work_struct worker;
+	oprf_add_sample_func *add_sample_f;
+	atomic_t ext_params;
+	unsigned long req_alert;
+	unsigned long loss_of_sample_data;
+	unsigned long invalid_entry_address;
+	unsigned long incorrect_sdbt_entry;
+	unsigned long sample_auth_change_alert;
+	unsigned int finish:1;
+	unsigned int oom:1;
+	unsigned int stop_mode:1;
+};
+
+struct data_entry {
+	unsigned int def:16;        /* 0-15  Data Entry Format           */
+	unsigned int R:4;           /* 16-19 reserved                    */
+	unsigned int U:4;           /* 20-23 Number of unique instruct.  */
+	unsigned int z:2;           /* zeros                             */
+	unsigned int T:1;           /* 26 PSW DAT mode                   */
+	unsigned int W:1;           /* 27 PSW wait state                 */
+	unsigned int P:1;           /* 28 PSW Problem state              */
+	unsigned int AS:2;          /* 29-30 PSW address-space control   */
+	unsigned int I:1;           /* 31 entry valid or invalid         */
+	unsigned int:16;
+	unsigned int prim_asn:16;   /* primary ASN                       */
+	unsigned long long ia;      /* Instruction Address               */
+	unsigned long long lpp;     /* Logical-Partition Program Param.  */
+	unsigned long long vpp;     /* Virtual-Machine Program Param.    */
+};
+
+struct trailer_entry {
+	unsigned int f:1;           /* 0 - Block Full Indicator          */
+	unsigned int a:1;           /* 1 - Alert request control         */
+	unsigned long:62;           /* 2 - 63: Reserved                  */
+	unsigned long overflow;     /* 64 - sample Overflow count        */
+	unsigned long timestamp;    /* 16 - time-stamp                   */
+	unsigned long timestamp1;   /*                                   */
+	unsigned long reserved1;    /* 32 -Reserved                      */
+	unsigned long reserved2;    /*                                   */
+	unsigned long progusage1;   /* 48 - reserved for programming use */
+	unsigned long progusage2;   /*                                   */
+};
+
+int hwsampler_setup(void);
+int hwsampler_shutdown(void);
+int hwsampler_allocate(unsigned long sdbt, unsigned long sdb);
+int hwsampler_deallocate(void);
+long hwsampler_query_min_interval(void);
+long hwsampler_query_max_interval(void);
+int hwsampler_start_all(unsigned long interval);
+int hwsampler_stop_all(void);
+int hwsampler_deactivate(unsigned int cpu);
+int hwsampler_activate(unsigned int cpu);
+unsigned long hwsampler_get_sample_overflow_count(unsigned int cpu);
+
+int smp_ctl_qsi(int);
+int smp_ctl_ssctl_deactivate(int);
+int smp_ctl_ssctl_stop(int);
+int smp_ctl_ssctl_enable_activate(int, unsigned long);
+
+int qsi(void *);
+
+void execute_qsi(void *);
+void execute_ssctl(void *);
+
+#endif /*HWSAMPLER_H_*/
+
drivers/s390/hwsampler/hwsampler-main.c new file +1155 -0

1,155 lines not on this page · raw message

drivers/s390/hwsampler/smpctl.c new file +170 -0

170 lines not on this page · raw message

arch/s390/Kconfig +22 -0
Index: linux-2.6/arch/s390/Kconfig
===================================================================
@@ -127,6 +127,7 @@ config S390
 	select ARCH_INLINE_WRITE_UNLOCK_BH
 	select ARCH_INLINE_WRITE_UNLOCK_IRQ
 	select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
+	select HAVE_HWSAMPLER
 
 config SCHED_OMIT_FRAME_POINTER
 	bool
@@ -618,6 +619,27 @@ config SECCOMP
 
 	  If unsure, say Y.
 
+config HWSAMPLER
+	tristate "Exploit CPUMF hardware sampling with OProfile"
+	depends on OPROFILE
+	depends on HAVE_HWSAMPLER
+	depends on 64BIT
+	select OPROFILE_HWSAMPLING_MODE
+	help
+	  Hardware (HW) sampling is a feature provided by z processor.
+	  The sampling process is implemented in hardware and millicode
+	  and thus does not affect the operating system being observed,
+	  apart from the required buffer memory that Linux kernel must
+	  provide.
+
+	  If unsure, say N.
+
+config HAVE_HWSAMPLER
+	bool
+
+config OPROFILE_HWSAMPLING_MODE
+	bool
+
 endmenu
 
 menu "Power Management"
arch/s390/oprofile/Makefile +6 -0
Index: linux-2.6/arch/s390/oprofile/Makefile
===================================================================
@@ -1,3 +1,4 @@
+obj-$(CONFIG_HWSAMPLER)        += hwsampler.o
 obj-$(CONFIG_OPROFILE) += oprofile.o
 
 DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
@@ -7,3 +8,8 @@ DRIVER_OBJS = $(addprefix ../../../drive
 		timer_int.o )
 
 oprofile-y				:= $(DRIVER_OBJS) init.o backtrace.o
+
+HW_SAMPLER_DRIVER_OBJS = $(addprefix ../../../drivers/s390/hwsampler/, \
+		hwsampler-main.o smpctl.o )
+
+hwsampler-y				:= $(HW_SAMPLER_DRIVER_OBJS)
#2 lore raw

Re: [patch 1/4] This patch adds support for hardware based sampling on System z processors (models z10 and up)

Quoted 1 line · On 20.12.10 08:05:42, [email protected] wrote:

From: [email protected]

This should include the real name like in your Signed-off-by tag.

You can fix this by reconfiguring git and recommitting your patches (git rebase -i ..., git commit --amend --reset-author).

Quoted 20 lines

The CPU Measurement Facility CPUMF is described in the z/Architecture Principles of Operation.

The patch introduces
- a new configuration option OPROFILE_HWSAMPLING_MODE
- a new kernel module hwsampler that controls all hardware sampling related operations as
- checking if hardware sampling feature is available
- ie: on System z models z10 and up, in LPAR mode only, and authorised during LPAR activation
- allocating memory for the hardware sampling feature
- starting/stopping hardware sampling
The hwsampler module will also depend on CONFIG_OPROFILE and CONFIG_64BIT.

All functions required to start and stop hardware sampling have to be invoked by the oprofile kernel module as provided by the other patches of this patch set.

In case hardware based sampling cannot be setup standard timer based sampling is used by OProfile.

Quoted 6 lines
---
 arch/s390/Kconfig                       |   22
 arch/s390/include/asm/hwsampler.h       |  130 +++
 arch/s390/oprofile/Makefile             |    6
 drivers/s390/hwsampler/hwsampler-main.c | 1155 ++++++++++++++++++++++++++++++++
 drivers/s390/hwsampler/smpctl.c         |  170 ++++

Is there a reason for splitting the code into two files? If we would merge hwsampler-main.c and smpctl.c we could make a lot functions static which simplifies the interface. We could also drop the hwsampler/ directory and put all in drivers/s390/hwsampler.c.

Another thing is, wouldn't all of this better be part of cpu initialization code? This is not really a driver, it only registers a cpu notifier. Do you need to build this as module? I leave this decision to the s390 maintainers.

arch/s390/include/asm/hwsampler.h

Quoted 6 lines
 5 files changed, 1483 insertions(+)

Index: linux-2.6/arch/s390/include/asm/hwsampler.h
===================================================================

arch/s390/include/asm/hwsampler.h new file +0 -0

This file should only contain definitions for the public interface. All structs should be private, defined in something like

drivers/s390/hwsampler.h

or so. All of them are only used in hwsampler-main.c or smpctl.c.

To avoid namespace collisions, add a prefix like hws_ to all symbols.

Quoted 47 lines
arch/s390/include/asm/hwsampler.h new file +46 -0
@@ -0,0 +1,130 @@
+/*
+ * CPUMF HW sampler structures and prototypes
+ *
+ *    Copyright IBM Corp. 2010
+ *    Author(s): Heinz Graalfs <[email protected]>
+ */
+
+#ifndef HWSAMPLER_H_
+#define HWSAMPLER_H_
+
+#include <linux/workqueue.h>
+
+struct qsi_info_block          /* QUERY SAMPLING information block  */
+{ /* Bit(s) */
+       unsigned int b0_13:14;      /* 0-13: zeros                       */
+       unsigned int as:1;          /* 14: sampling authorisation control*/
+       unsigned int b15_21:7;      /* 15-21: zeros                      */
+       unsigned int es:1;          /* 22: sampling enable control       */
+       unsigned int b23_29:7;      /* 23-29: zeros                      */
+       unsigned int cs:1;          /* 30: sampling activation control   */
+       unsigned int:1;             /* 31: reserved                      */
+       unsigned int bsdes:16;      /* 4-5: size of sampling entry       */
+       unsigned int:16;            /* 6-7: reserved                     */
+       unsigned long min_sampl_rate; /* 8-15: minimum sampling interval */
+       unsigned long max_sampl_rate; /* 16-23: maximum sampling interval*/
+       unsigned long tear;         /* 24-31: TEAR contents              */
+       unsigned long dear;         /* 32-39: DEAR contents              */
+       unsigned int rsvrd0;        /* 40-43: reserved                   */
+       unsigned int cpu_speed;     /* 44-47: CPU speed                  */
+       unsigned long long rsvrd1;  /* 48-55: reserved                   */
+       unsigned long long rsvrd2;  /* 56-63: reserved                   */
+};
+
+struct ssctl_request_block     /* SET SAMPLING CONTROLS req block   */
+{ /* bytes 0 - 7  Bit(s) */
+       unsigned int s:1;           /* 0: maximum buffer indicator       */
+       unsigned int h:1;           /* 1: part. level reserved for VM use*/
+       unsigned long b2_53:52;     /* 2-53: zeros                       */
+       unsigned int es:1;          /* 54: sampling enable control       */
+       unsigned int b55_61:7;      /* 55-61: - zeros                    */
+       unsigned int cs:1;          /* 62: sampling activation control   */
+       unsigned int b63:1;         /* 63: zero                          */
+       unsigned long interval;     /* 8-15: sampling interval           */
+       unsigned long tear;         /* 16-23: TEAR contents              */
+       unsigned long dear;         /* 24-31: DEAR contents              */
+       /* 32-63:                                                        */

arch/s390/include/asm/hwsampler.h line 56

Quoted 10 lines
arch/s390/include/asm/hwsampler.h new file +10 -0
+       unsigned long rsvrd1;       /* reserved                          */
+       unsigned long rsvrd2;       /* reserved                          */
+       unsigned long rsvrd3;       /* reserved                          */
+       unsigned long rsvrd4;       /* reserved                          */
+};
+
+typedef void oprf_add_sample_func(unsigned long pc,
+       struct pt_regs * const regs,
+       unsigned long event, int is_kernel,
+       struct task_struct *task);

Don't use typedefs.

Quoted 52 lines
arch/s390/include/asm/hwsampler.h new file +52 -0
+
+struct cpu_buffer {
+       unsigned long first_sdbt;       /* @ of 1st SDB-Table for this CP*/
+       unsigned long worker_entry;
+       unsigned long sample_overflow;  /* taken from SDB ...            */
+       struct qsi_info_block qsi;
+       struct ssctl_request_block ssctl;
+       struct work_struct worker;
+       oprf_add_sample_func *add_sample_f;
+       atomic_t ext_params;
+       unsigned long req_alert;
+       unsigned long loss_of_sample_data;
+       unsigned long invalid_entry_address;
+       unsigned long incorrect_sdbt_entry;
+       unsigned long sample_auth_change_alert;
+       unsigned int finish:1;
+       unsigned int oom:1;
+       unsigned int stop_mode:1;
+};
+
+struct data_entry {
+       unsigned int def:16;        /* 0-15  Data Entry Format           */
+       unsigned int R:4;           /* 16-19 reserved                    */
+       unsigned int U:4;           /* 20-23 Number of unique instruct.  */
+       unsigned int z:2;           /* zeros                             */
+       unsigned int T:1;           /* 26 PSW DAT mode                   */
+       unsigned int W:1;           /* 27 PSW wait state                 */
+       unsigned int P:1;           /* 28 PSW Problem state              */
+       unsigned int AS:2;          /* 29-30 PSW address-space control   */
+       unsigned int I:1;           /* 31 entry valid or invalid         */
+       unsigned int:16;
+       unsigned int prim_asn:16;   /* primary ASN                       */
+       unsigned long long ia;      /* Instruction Address               */
+       unsigned long long lpp;     /* Logical-Partition Program Param.  */
+       unsigned long long vpp;     /* Virtual-Machine Program Param.    */
+};
+
+struct trailer_entry {
+       unsigned int f:1;           /* 0 - Block Full Indicator          */
+       unsigned int a:1;           /* 1 - Alert request control         */
+       unsigned long:62;           /* 2 - 63: Reserved                  */
+       unsigned long overflow;     /* 64 - sample Overflow count        */
+       unsigned long timestamp;    /* 16 - time-stamp                   */
+       unsigned long timestamp1;   /*                                   */
+       unsigned long reserved1;    /* 32 -Reserved                      */
+       unsigned long reserved2;    /*                                   */
+       unsigned long progusage1;   /* 48 - reserved for programming use */
+       unsigned long progusage2;   /*                                   */
+};
+
+int hwsampler_setup(void);
+int hwsampler_shutdown(void);

arch/s390/include/asm/hwsampler.h line 118

Quoted 10 lines
arch/s390/include/asm/hwsampler.h new file +10 -0
+int hwsampler_allocate(unsigned long sdbt, unsigned long sdb);
+int hwsampler_deallocate(void);
+long hwsampler_query_min_interval(void);
+long hwsampler_query_max_interval(void);
+int hwsampler_start_all(unsigned long interval);
+int hwsampler_stop_all(void);
+int hwsampler_deactivate(unsigned int cpu);
+int hwsampler_activate(unsigned int cpu);
+unsigned long hwsampler_get_sample_overflow_count(unsigned int cpu);
+

All the following functions should have a prefix (e.g. hws_):

arch/s390/include/asm/hwsampler.h line 127

Quoted 9 lines
arch/s390/include/asm/hwsampler.h new file +9 -0
+int smp_ctl_qsi(int);
+int smp_ctl_ssctl_deactivate(int);
+int smp_ctl_ssctl_stop(int);
+int smp_ctl_ssctl_enable_activate(int, unsigned long);
+
+int qsi(void *);
+
+void execute_qsi(void *);
+void execute_ssctl(void *);

Many functions above are for internal use only, these should be removed from this interface and made static.

Quoted 7 lines
arch/s390/include/asm/hwsampler.h new file +3 -0
+
+#endif /*HWSAMPLER_H_*/
+

Index: linux-2.6/drivers/s390/hwsampler/hwsampler-main.c
===================================================================

drivers/s390/hwsampler/hwsampler-main.c line 440

Quoted 7 lines
drivers/s390/hwsampler/hwsampler-main.c new file +6 -0
 
+static int __cpuinit hws_cpu_callback(struct notifier_block *nfb,
+       unsigned long action, void *hcpu)
+{
+       /* We do not have sampler space available for all possible CPUs.
+          All CPUs should be online when hw sampling is activated. */
+       return NOTIFY_BAD;

Is this to prevent bringing cpus on-/offline?

Quoted 1 line

+}

[...]

Quoted 18 lines
drivers/s390/hwsampler/hwsampler-main.c new file +18 -0
+static int __init hwsampler_init(void)
+{
+       hws_state = HWS_INIT;
+       register_cpu_notifier(&hws_cpu_notifier);
+       return 0;
+}
+
+static void __exit hwsampler_exit(void)
+{
+       unregister_cpu_notifier(&hws_cpu_notifier);
+}
+
+module_init(hwsampler_init);
+module_exit(hwsampler_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Heinz Graalfs <[email protected]>");
+MODULE_DESCRIPTION("IBM CPUMF Customer Mode Sampling Kernel Module");

[...]

Quoted 50 lines

Index: linux-2.6/arch/s390/Kconfig
===================================================================

arch/s390/Kconfig +22 -0
@@ -127,6 +127,7 @@ config S390
        select ARCH_INLINE_WRITE_UNLOCK_BH
        select ARCH_INLINE_WRITE_UNLOCK_IRQ
        select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
+       select HAVE_HWSAMPLER
 
 config SCHED_OMIT_FRAME_POINTER
        bool
@@ -618,6 +619,27 @@ config SECCOMP
 
          If unsure, say Y.
 
+config HWSAMPLER
+       tristate "Exploit CPUMF hardware sampling with OProfile"
+       depends on OPROFILE
+       depends on HAVE_HWSAMPLER
+       depends on 64BIT
+       select OPROFILE_HWSAMPLING_MODE
+       help
+         Hardware (HW) sampling is a feature provided by z processor.
+         The sampling process is implemented in hardware and millicode
+         and thus does not affect the operating system being observed,
+         apart from the required buffer memory that Linux kernel must
+         provide.
+
+         If unsure, say N.
+
+config HAVE_HWSAMPLER
+       bool
+
+config OPROFILE_HWSAMPLING_MODE
+       bool
+
 endmenu
 
 menu "Power Management"

Index: linux-2.6/arch/s390/oprofile/Makefile
===================================================================

arch/s390/oprofile/Makefile +1 -0
@@ -1,3 +1,4 @@
+obj-$(CONFIG_HWSAMPLER)        += hwsampler.o
 obj-$(CONFIG_OPROFILE) += oprofile.o
 
 DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \

arch/s390/oprofile/Makefile DRIVER_OBJS = $(addprefix ../../../drive · line 15

Quoted 8 lines
arch/s390/oprofile/Makefile +5 -0
@@ -7,3 +8,8 @@ DRIVER_OBJS = $(addprefix ../../../drive
                timer_int.o )
 
 oprofile-y                             := $(DRIVER_OBJS) init.o backtrace.o
+
+HW_SAMPLER_DRIVER_OBJS = $(addprefix ../../../drivers/s390/hwsampler/, \
+               hwsampler-main.o smpctl.o )
+
+hwsampler-y                            := $(HW_SAMPLER_DRIVER_OBJS)

Have you tried building this as a module. Not really sure, but I think it should be

hwsampler-$(CONFIG_HWSAMPLER) := ...

See also my statement above about putting this to cpu init code instead of having a driver for it.

-Robert

Quoted 2 lines
Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#3 lore raw

Re: [patch 1/4] This patch adds support for hardware based sampling on System z processors (models z10 and up)

Hello Robert,

please find below (and in the following 2 mails) my answers to your comments.

The patch-set will follow soon.

Heinz

Quoted 35 lines · Robert Richter
Quoted 1 line · On 20.12.10 08:05:42, [email protected] wrote:

From: [email protected]

This should include the real name like in your Signed-off-by tag.

You can fix this by reconfiguring git and recommitting your patches (git rebase -i ..., git commit --amend --reset-author).

Quoted 20 lines

The CPU Measurement Facility CPUMF is described in the z/Architecture Principles of Operation.

The patch introduces
- a new configuration option OPROFILE_HWSAMPLING_MODE
- a new kernel module hwsampler that controls all hardware sampling related operations as
- checking if hardware sampling feature is available
- ie: on System z models z10 and up, in LPAR mode only, and authorised during LPAR activation
- allocating memory for the hardware sampling feature
- starting/stopping hardware sampling
The hwsampler module will also depend on CONFIG_OPROFILE and CONFIG_64BIT.

All functions required to start and stop hardware sampling have to be invoked by the oprofile kernel module as provided by the other patches of this patch set.

In case hardware based sampling cannot be setup standard timer based sampling is used by OProfile.

Quoted 6 lines
---
 arch/s390/Kconfig                       |   22
 arch/s390/include/asm/hwsampler.h       |  130 +++
 arch/s390/oprofile/Makefile             |    6
 drivers/s390/hwsampler/hwsampler-main.c | 1155 ++++++++++++++++++++++++++++++++
 drivers/s390/hwsampler/smpctl.c         |  170 ++++
Quoted 4 lines

Is there a reason for splitting the code into two files? If we would merge hwsampler-main.c and smpctl.c we could make a lot functions static which simplifies the interface. We could also drop the hwsampler/ directory and put all in drivers/s390/hwsampler.c.

I merged smpctl.c contents into new hwsampler.c, hwsampler.c is now located in arch/s390/oprofile.

As you proposed I integrated everything in the oprofile kernel module.

Quoted 16 lines

Another thing is, wouldn't all of this better be part of cpu initialization code? This is not really a driver, it only registers a cpu notifier. Do you need to build this as module? I leave this decision to the s390 maintainers.

arch/s390/include/asm/hwsampler.h

Quoted 6 lines
 5 files changed, 1483 insertions(+)

Index: linux-2.6/arch/s390/include/asm/hwsampler.h
===================================================================

arch/s390/include/asm/hwsampler.h new file +0 -0

This file should only contain definitions for the public interface. All structs should be private, defined in something like

Quoted 1 line

drivers/s390/hwsampler.h

I moved the structs to drivers/s390/hwsampler.h

Quoted 5 lines

or so. All of them are only used in hwsampler-main.c or smpctl.c.

To avoid namespace collisions, add a prefix like hws_ to all symbols.

OK, done

Quoted 51 lines
Quoted 51 lines
arch/s390/include/asm/hwsampler.h new file +50 -0
@@ -0,0 +1,130 @@
+/*
+ * CPUMF HW sampler structures and prototypes
+ *
+ *    Copyright IBM Corp. 2010
+ *    Author(s): Heinz Graalfs <[email protected]>
+ */
+
+#ifndef HWSAMPLER_H_
+#define HWSAMPLER_H_
+
+#include <linux/workqueue.h>
+
+struct qsi_info_block          /* QUERY SAMPLING information block  */
+{ /* Bit(s) */
+       unsigned int b0_13:14;      /* 0-13: zeros                       */
+       unsigned int as:1;          /* 14: sampling authorisation control*/
+       unsigned int b15_21:7;      /* 15-21: zeros                      */
+       unsigned int es:1;          /* 22: sampling enable control       */
+       unsigned int b23_29:7;      /* 23-29: zeros                      */
+       unsigned int cs:1;          /* 30: sampling activation control   */
+       unsigned int:1;             /* 31: reserved                      */
+       unsigned int bsdes:16;      /* 4-5: size of sampling entry       */
+       unsigned int:16;            /* 6-7: reserved                     */
+       unsigned long min_sampl_rate; /* 8-15: minimum sampling interval */
+       unsigned long max_sampl_rate; /* 16-23: maximum sampling interval*/
+       unsigned long tear;         /* 24-31: TEAR contents              */
+       unsigned long dear;         /* 32-39: DEAR contents              */
+       unsigned int rsvrd0;        /* 40-43: reserved                   */
+       unsigned int cpu_speed;     /* 44-47: CPU speed                  */
+       unsigned long long rsvrd1;  /* 48-55: reserved                   */
+       unsigned long long rsvrd2;  /* 56-63: reserved                   */
+};
+
+struct ssctl_request_block     /* SET SAMPLING CONTROLS req block   */
+{ /* bytes 0 - 7  Bit(s) */
+       unsigned int s:1;           /* 0: maximum buffer indicator       */
+       unsigned int h:1;           /* 1: part. level reserved for VM use*/
+       unsigned long b2_53:52;     /* 2-53: zeros                       */
+       unsigned int es:1;          /* 54: sampling enable control       */
+       unsigned int b55_61:7;      /* 55-61: - zeros                    */
+       unsigned int cs:1;          /* 62: sampling activation control   */
+       unsigned int b63:1;         /* 63: zero                          */
+       unsigned long interval;     /* 8-15: sampling interval           */
+       unsigned long tear;         /* 16-23: TEAR contents              */
+       unsigned long dear;         /* 24-31: DEAR contents              */
+       /* 32-63:                                                        */
+       unsigned long rsvrd1;       /* reserved                          */
+       unsigned long rsvrd2;       /* reserved                          */
+       unsigned long rsvrd3;       /* reserved                          */
+       unsigned long rsvrd4;       /* reserved                          */
Quoted 8 lines

arch/s390/include/asm/hwsampler.h line 56

Quoted 6 lines
arch/s390/include/asm/hwsampler.h new file +6 -0
+};
+
+typedef void oprf_add_sample_func(unsigned long pc,
+       struct pt_regs * const regs,
+       unsigned long event, int is_kernel,
+       struct task_struct *task);

Don't use typedefs.

OK, done

Quoted 57 lines
Quoted 57 lines
arch/s390/include/asm/hwsampler.h new file +56 -0
+
+struct cpu_buffer {
+       unsigned long first_sdbt;       /* @ of 1st SDB-Table for this CP*/
+       unsigned long worker_entry;
+       unsigned long sample_overflow;  /* taken from SDB ...            */
+       struct qsi_info_block qsi;
+       struct ssctl_request_block ssctl;
+       struct work_struct worker;
+       oprf_add_sample_func *add_sample_f;
+       atomic_t ext_params;
+       unsigned long req_alert;
+       unsigned long loss_of_sample_data;
+       unsigned long invalid_entry_address;
+       unsigned long incorrect_sdbt_entry;
+       unsigned long sample_auth_change_alert;
+       unsigned int finish:1;
+       unsigned int oom:1;
+       unsigned int stop_mode:1;
+};
+
+struct data_entry {
+       unsigned int def:16;        /* 0-15  Data Entry Format           */
+       unsigned int R:4;           /* 16-19 reserved                    */
+       unsigned int U:4;           /* 20-23 Number of unique instruct.  */
+       unsigned int z:2;           /* zeros                             */
+       unsigned int T:1;           /* 26 PSW DAT mode                   */
+       unsigned int W:1;           /* 27 PSW wait state                 */
+       unsigned int P:1;           /* 28 PSW Problem state              */
+       unsigned int AS:2;          /* 29-30 PSW address-space control   */
+       unsigned int I:1;           /* 31 entry valid or invalid         */
+       unsigned int:16;
+       unsigned int prim_asn:16;   /* primary ASN                       */
+       unsigned long long ia;      /* Instruction Address               */
+       unsigned long long lpp;     /* Logical-Partition Program Param.  */
+       unsigned long long vpp;     /* Virtual-Machine Program Param.    */
+};
+
+struct trailer_entry {
+       unsigned int f:1;           /* 0 - Block Full Indicator          */
+       unsigned int a:1;           /* 1 - Alert request control         */
+       unsigned long:62;           /* 2 - 63: Reserved                  */
+       unsigned long overflow;     /* 64 - sample Overflow count        */
+       unsigned long timestamp;    /* 16 - time-stamp                   */
+       unsigned long timestamp1;   /*                                   */
+       unsigned long reserved1;    /* 32 -Reserved                      */
+       unsigned long reserved2;    /*                                   */
+       unsigned long progusage1;   /* 48 - reserved for programming use */
+       unsigned long progusage2;   /*                                   */
+};
+
+int hwsampler_setup(void);
+int hwsampler_shutdown(void);
+int hwsampler_allocate(unsigned long sdbt, unsigned long sdb);
+int hwsampler_deallocate(void);
+long hwsampler_query_min_interval(void);
+long hwsampler_query_max_interval(void);
Quoted 8 lines

arch/s390/include/asm/hwsampler.h line 118

Quoted 6 lines
arch/s390/include/asm/hwsampler.h new file +6 -0
+int hwsampler_start_all(unsigned long interval);
+int hwsampler_stop_all(void);
+int hwsampler_deactivate(unsigned int cpu);
+int hwsampler_activate(unsigned int cpu);
+unsigned long hwsampler_get_sample_overflow_count(unsigned int cpu);
+

All the following functions should have a prefix (e.g. hws_):

all smp_ functions are now static in hwsampler.c

Quoted 14 lines

arch/s390/include/asm/hwsampler.h line 127

Quoted 10 lines
arch/s390/include/asm/hwsampler.h new file +9 -0
+int smp_ctl_qsi(int);
+int smp_ctl_ssctl_deactivate(int);
+int smp_ctl_ssctl_stop(int);
+int smp_ctl_ssctl_enable_activate(int, unsigned long);
+
+int qsi(void *);
+
+void execute_qsi(void *);
+void execute_ssctl(void *);

Many functions above are for internal use only, these should be removed from this interface and made static.

all structs moved to arch/s390/oprofile/hwsampler.h

Quoted 8 lines
Quoted 7 lines
arch/s390/include/asm/hwsampler.h new file +3 -0
+
+#endif /*HWSAMPLER_H_*/
+

Index: linux-2.6/drivers/s390/hwsampler/hwsampler-main.c
===================================================================

drivers/s390/hwsampler/hwsampler-main.c new file +0 -0
Quoted 8 lines

drivers/s390/hwsampler/hwsampler-main.c line 440

Quoted 6 lines
drivers/s390/hwsampler/hwsampler-main.c new file +6 -0
+static int __cpuinit hws_cpu_callback(struct notifier_block *nfb,
+       unsigned long action, void *hcpu)
+{
+       /* We do not have sampler space available for all possible CPUs.
+          All CPUs should be online when hw sampling is activated. */
+       return NOTIFY_BAD;

Is this to prevent bringing cpus on-/offline?

yes, we don't allow cpu varyon/off during hardware sampling

Quoted 78 lines
Quoted 2 lines

+}

[...]

Quoted 18 lines
drivers/s390/hwsampler/hwsampler-main.c new file +18 -0
+static int __init hwsampler_init(void)
+{
+       hws_state = HWS_INIT;
+       register_cpu_notifier(&hws_cpu_notifier);
+       return 0;
+}
+
+static void __exit hwsampler_exit(void)
+{
+       unregister_cpu_notifier(&hws_cpu_notifier);
+}
+
+module_init(hwsampler_init);
+module_exit(hwsampler_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Heinz Graalfs <[email protected]>");
+MODULE_DESCRIPTION("IBM CPUMF Customer Mode Sampling Kernel Module");

[...]

Quoted 52 lines

Index: linux-2.6/arch/s390/Kconfig
===================================================================

arch/s390/Kconfig +22 -0
@@ -127,6 +127,7 @@ config S390
        select ARCH_INLINE_WRITE_UNLOCK_BH
        select ARCH_INLINE_WRITE_UNLOCK_IRQ
        select ARCH_INLINE_WRITE_UNLOCK_IRQRESTORE
+       select HAVE_HWSAMPLER
 
 config SCHED_OMIT_FRAME_POINTER
        bool
@@ -618,6 +619,27 @@ config SECCOMP
 
          If unsure, say Y.
 
+config HWSAMPLER
+       tristate "Exploit CPUMF hardware sampling with OProfile"
+       depends on OPROFILE
+       depends on HAVE_HWSAMPLER
+       depends on 64BIT
+       select OPROFILE_HWSAMPLING_MODE
+       help
+         Hardware (HW) sampling is a feature provided by z processor.
+         The sampling process is implemented in hardware and millicode
+         and thus does not affect the operating system being observed,
+         apart from the required buffer memory that Linux kernel must
+         provide.
+
+         If unsure, say N.
+
+config HAVE_HWSAMPLER
+       bool
+
+config OPROFILE_HWSAMPLING_MODE
+       bool
+
 endmenu
 
 menu "Power Management"

Index: linux-2.6/arch/s390/oprofile/Makefile
===================================================================

arch/s390/oprofile/Makefile +1 -0
@@ -1,3 +1,4 @@
+obj-$(CONFIG_HWSAMPLER)        += hwsampler.o
 obj-$(CONFIG_OPROFILE) += oprofile.o
 
 DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
@@ -7,3 +8,8 @@ DRIVER_OBJS = $(addprefix ../../../drive
                timer_int.o )
 
Quoted 9 lines

arch/s390/oprofile/Makefile DRIVER_OBJS = $(addprefix ../../../drive · line 15

Quoted 6 lines
arch/s390/oprofile/Makefile +5 -0
 oprofile-y                             := $(DRIVER_OBJS) init.o backtrace.o
+
+HW_SAMPLER_DRIVER_OBJS = $(addprefix ../../../drivers/s390/hwsampler/, \
+               hwsampler-main.o smpctl.o )
+
+hwsampler-y                            := $(HW_SAMPLER_DRIVER_OBJS)

Have you tried building this as a module. Not really sure, but I think it should be

see above

Quoted 10 lines

hwsampler-$(CONFIG_HWSAMPLER) := ...

See also my statement above about putting this to cpu init code instead of having a driver for it.

-Robert

Quoted 2 lines
#4 lore raw

PATCH 2/4 This patch enhances OProfile to support System zs hardware sampling feature

In reply to [email protected]

From: [email protected]

OProfile is enhanced to export all files for controlling System z's hardware sampling, and to invoke hwsampler exported functions to initialize and use System z's hardware sampling.

The patch invokes hwsampler_setup() during oprofile init and exports following hwsampler files under oprofilefs if hwsampler's setup succeeded:

A new directory for hardware sampling based files

/dev/oprofile/hwsampling/

The userland daemon must explicitly write to the following files to disable (or enable) hardware based sampling

/dev/oprofile/hwsampling/hwsampler

to modify the actual sampling rate

/dev/oprofile/hwsampling/hw_interval

to modify the amount of sampling memory (measured in 4K pages)

/dev/oprofile/hwsampling/hw_sdbt_blocks

The following files are read only and show
the possible minimum sampling rate

/dev/oprofile/hwsampling/hw_min_interval

the possible maximum sampling rate

/dev/oprofile/hwsampling/hw_max_interval

The patch splits the oprofile_timer_[init/exit] function so that it can be also called through user context (oprofilefs) to avoid kernel oops.

---
 arch/s390/oprofile/Makefile          |    1 
 arch/s390/oprofile/hwsampler_files.c |  120 +++++++++++++++++++++++++++++++++++
 arch/s390/oprofile/init.c            |   35 ++++++++++
 drivers/oprofile/oprof.c             |   37 ++++++++++
 drivers/oprofile/oprof.h             |    2 
 drivers/oprofile/timer_int.c         |   16 +++-
 include/linux/oprofile.h             |   15 ++++
 7 files changed, 223 insertions(+), 3 deletions(-)
arch/s390/oprofile/Makefile +1 -0
Index: linux-2.6/arch/s390/oprofile/Makefile
===================================================================
@@ -8,6 +8,7 @@ DRIVER_OBJS = $(addprefix ../../../drive
 		timer_int.o )
 
 oprofile-y				:= $(DRIVER_OBJS) init.o backtrace.o
+oprofile-y				+= $(if $(CONFIG_HWSAMPLER), hwsampler_files.o,)
 
 HW_SAMPLER_DRIVER_OBJS = $(addprefix ../../../drivers/s390/hwsampler/, \
 		hwsampler-main.o smpctl.o )
arch/s390/oprofile/hwsampler_files.c new file +120 -0

120 lines not on this page · raw message

drivers/oprofile/oprof.c +37 -0
Index: linux-2.6/drivers/oprofile/oprof.c
===================================================================
@@ -239,10 +239,43 @@ int oprofile_set_ulong(unsigned long *ad
 	return err;
 }
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+int oprofile_set_hwsampler(unsigned long val)
+{
+	int err = 0;
+
+	mutex_lock(&start_mutex);
+
+	if (oprofile_started) {
+		err = -EBUSY;
+		goto out;
+	}
+
+	switch (val) {
+	case 1:
+		/* Switch to hardware sampling. */
+		__oprofile_timer_exit();
+		err = oprofile_arch_set_hwsampler(&oprofile_ops);
+		break;
+	case 0:
+		printk(KERN_INFO "oprofile: using timer interrupt.\n");
+		err = __oprofile_timer_init(&oprofile_ops);
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+out:
+	mutex_unlock(&start_mutex);
+	return err;
+}
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
+
 static int __init oprofile_init(void)
 {
 	int err;
 
+	memset(&oprofile_ops, 0, sizeof(oprofile_ops));
 	err = oprofile_arch_init(&oprofile_ops);
 	if (err < 0 || timer) {
 		printk(KERN_INFO "oprofile: using timer interrupt.\n");
@@ -250,6 +283,10 @@ static int __init oprofile_init(void)
 		if (err)
 			return err;
 	}
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+	else if (err == 0)
+		oprofile_arch_set_hwsampler(&oprofile_ops);
+#endif
 	return oprofilefs_register();
 }
 
drivers/oprofile/oprof.h +2 -0
Index: linux-2.6/drivers/oprofile/oprof.h
===================================================================
@@ -35,7 +35,9 @@ struct dentry;
 
 void oprofile_create_files(struct super_block *sb, struct dentry *root);
 int oprofile_timer_init(struct oprofile_operations *ops);
+int __oprofile_timer_init(struct oprofile_operations *ops);
 void oprofile_timer_exit(void);
+void __oprofile_timer_exit(void);
 
 int oprofile_set_ulong(unsigned long *addr, unsigned long val);
 int oprofile_set_timeout(unsigned long time);
drivers/oprofile/timer_int.c +13 -3
Index: linux-2.6/drivers/oprofile/timer_int.c
===================================================================
@@ -97,14 +97,13 @@ static struct notifier_block __refdata o
 	.notifier_call = oprofile_cpu_notify,
 };
 
-int __init oprofile_timer_init(struct oprofile_operations *ops)
+int  __oprofile_timer_init(struct oprofile_operations *ops)
 {
 	int rc;
 
 	rc = register_hotcpu_notifier(&oprofile_cpu_notifier);
 	if (rc)
 		return rc;
-	ops->create_files = NULL;
 	ops->setup = NULL;
 	ops->shutdown = NULL;
 	ops->start = oprofile_hrtimer_start;
@@ -113,7 +112,18 @@ int __init oprofile_timer_init(struct op
 	return 0;
 }
 
-void __exit oprofile_timer_exit(void)
+int __init oprofile_timer_init(struct oprofile_operations *ops)
+{
+	return __oprofile_timer_init(ops);
+}
+
+void __oprofile_timer_exit(void)
 {
 	unregister_hotcpu_notifier(&oprofile_cpu_notifier);
 }
+
+void __exit oprofile_timer_exit(void)
+{
+	__oprofile_timer_exit();
+}
+
include/linux/oprofile.h +15 -0
Index: linux-2.6/include/linux/oprofile.h
===================================================================
@@ -89,6 +89,21 @@ int oprofile_arch_init(struct oprofile_o
  */
 void oprofile_arch_exit(void);
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+/**
+ * setup hardware sampler for oprofiling.
+ */
+
+int oprofile_set_hwsampler(unsigned long);
+
+/**
+ * hardware sampler module initialization for the s390 arch
+ */
+
+int oprofile_arch_set_hwsampler(struct oprofile_operations *ops);
+
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
+
 /**
  * Add a sample. This may be called from any context.
  */
arch/s390/oprofile/init.c +35 -0
Index: linux-2.6/arch/s390/oprofile/init.c
===================================================================
@@ -11,16 +11,51 @@
 #include <linux/oprofile.h>
 #include <linux/init.h>
 #include <linux/errno.h>
+#include <linux/fs.h>
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+#include <asm/hwsampler.h>
+
+extern int oprofile_create_hwsampling_files(struct super_block *sb,
+							struct dentry *root);
+
+extern unsigned long oprofile_min_interval;
+extern unsigned long oprofile_max_interval;
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
 
 extern void s390_backtrace(struct pt_regs * const regs, unsigned int depth);
 
 int __init oprofile_arch_init(struct oprofile_operations* ops)
 {
 	ops->backtrace = s390_backtrace;
+
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+	if (hwsampler_setup())
+		return -ENODEV;
+
+	/*
+	 * create hwsampler files only if hwsampler_setup() succeeds.
+	 */
+	ops->create_files = oprofile_create_hwsampling_files;
+	oprofile_min_interval = hwsampler_query_min_interval();
+	if (oprofile_min_interval < 0) {
+		oprofile_min_interval = 0;
+		return -ENODEV;
+	}
+	oprofile_max_interval = hwsampler_query_max_interval();
+	if (oprofile_max_interval < 0) {
+		oprofile_max_interval = 0;
+		return -ENODEV;
+	}
+	return 0;
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
+
 	return -ENODEV;
 }
 
 void oprofile_arch_exit(void)
 {
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+	hwsampler_shutdown();
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
 }
#5 lore raw

Re: [patch 2/4] This patch enhances OProfile to support System zs hardware sampling feature

Quoted 37 lines · On 20.12.10 08:05:43, [email protected] wrote:

From: [email protected]

OProfile is enhanced to export all files for controlling System z's hardware sampling, and to invoke hwsampler exported functions to initialize and use System z's hardware sampling.

The patch invokes hwsampler_setup() during oprofile init and exports following hwsampler files under oprofilefs if hwsampler's setup succeeded:

A new directory for hardware sampling based files

/dev/oprofile/hwsampling/

The userland daemon must explicitly write to the following files to disable (or enable) hardware based sampling

/dev/oprofile/hwsampling/hwsampler

to modify the actual sampling rate

/dev/oprofile/hwsampling/hw_interval

to modify the amount of sampling memory (measured in 4K pages)

/dev/oprofile/hwsampling/hw_sdbt_blocks

The following files are read only and show
the possible minimum sampling rate

/dev/oprofile/hwsampling/hw_min_interval

the possible maximum sampling rate

/dev/oprofile/hwsampling/hw_max_interval

The patch splits the oprofile_timer_[init/exit] function so that it can be also called through user context (oprofilefs) to avoid kernel oops.

Quoted 6 lines
---
 arch/s390/oprofile/Makefile          |    1
 arch/s390/oprofile/hwsampler_files.c |  120 +++++++++++++++++++++++++++++++++++

I would rather see a file hwsampler.c here that contains all oprofile hwsampler code in it and also sets up a struct oprofile_operations* ops.

Doing so, most of global functions and variables below can be made static.

Quoted 1 line
 arch/s390/oprofile/init.c            |   35 ++++++++++

We should find a better solution than changing all those files only for a single architecture:

Quoted 4 lines
 drivers/oprofile/oprof.c             |   37 ++++++++++
 drivers/oprofile/oprof.h             |    2
 drivers/oprofile/timer_int.c         |   16 +++-
 include/linux/oprofile.h             |   15 ++++

I want to see most of this in arch/s390.

Quoted 29 lines
 7 files changed, 223 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/s390/oprofile/Makefile
===================================================================

arch/s390/oprofile/Makefile +1 -0
@@ -8,6 +8,7 @@ DRIVER_OBJS = $(addprefix ../../../drive
                timer_int.o )
 
 oprofile-y                             := $(DRIVER_OBJS) init.o backtrace.o
+oprofile-y                             += $(if $(CONFIG_HWSAMPLER), hwsampler_files.o,)
 
 HW_SAMPLER_DRIVER_OBJS = $(addprefix ../../../drivers/s390/hwsampler/, \
                hwsampler-main.o smpctl.o )

Index: linux-2.6/arch/s390/oprofile/hwsampler_files.c
===================================================================

arch/s390/oprofile/hwsampler_files.c new file +10 -0
@@ -0,0 +1,120 @@
+/**
+ * arch/s390/oprofile/hwsampler_files.c
+ *
+ * Copyright IBM Corp. 2010
+ * Author: Mahesh Salgaonkar ([email protected])
+ */
+#include <linux/oprofile.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+

arch/s390/oprofile/hwsampler_files.c line 20

Quoted 10 lines
arch/s390/oprofile/hwsampler_files.c new file +10 -0
+#include <asm/hwsampler.h>
+
+#define DEFAULT_INTERVAL       4096
+
+#define DEFAULT_SDBT_BLOCKS    1
+#define DEFAULT_SDB_BLOCKS     511
+
+static unsigned long oprofile_hw_interval = DEFAULT_INTERVAL;
+unsigned long oprofile_min_interval;
+unsigned long oprofile_max_interval;

This could be static.

arch/s390/oprofile/hwsampler_files.c line 31

Quoted 11 lines
arch/s390/oprofile/hwsampler_files.c new file +11 -0
+
+static unsigned long oprofile_sdbt_blocks = DEFAULT_SDBT_BLOCKS;
+static unsigned long oprofile_sdb_blocks = DEFAULT_SDB_BLOCKS;
+
+static unsigned long oprofile_hwsampler;
+
+static int oprofile_hwsampler_start(void)
+{
+       int retval;
+
+       printk(KERN_INFO "oprofile_hwsampler_start\n");

This looks like a debug msg.

arch/s390/oprofile/hwsampler_files.c line 44

Quoted 13 lines
arch/s390/oprofile/hwsampler_files.c new file +13 -0
+
+       retval = hwsampler_allocate(oprofile_sdbt_blocks, oprofile_sdb_blocks);
+       if (retval)
+               return retval;
+
+       retval = hwsampler_start_all(oprofile_hw_interval);
+
+       return retval;
+}
+
+static void oprofile_hwsampler_stop(void)
+{
+       printk(KERN_INFO "oprofile_hwsampler_stop\n");

Same here.

arch/s390/oprofile/hwsampler_files.c line 56

Quoted 12 lines
arch/s390/oprofile/hwsampler_files.c new file +12 -0
+
+       hwsampler_stop_all();
+       hwsampler_deallocate();
+       return;
+}
+
+int oprofile_arch_set_hwsampler(struct oprofile_operations *ops)
+{
+       printk(KERN_INFO "oprofile: using hardware sampling\n");
+       ops->start = oprofile_hwsampler_start;
+       ops->stop = oprofile_hwsampler_stop;
+       ops->cpu_type = "timer";

Wouldn't it be better to have a different cpu_type string here, I don't think the oprofilefs interface is exactly the same as for timer mode. How the daemon distinguishs between both modes?

Quoted 32 lines
arch/s390/oprofile/hwsampler_files.c new file +32 -0
+
+       return 0;
+}
+
+static ssize_t hwsampler_read(struct file *file, char __user *buf,
+               size_t count, loff_t *offset)
+{
+       return oprofilefs_ulong_to_user(oprofile_hwsampler, buf, count, offset);
+}
+
+static ssize_t hwsampler_write(struct file *file, char const __user *buf,
+               size_t count, loff_t *offset)
+{
+       unsigned long val;
+       int retval;
+
+       if (*offset)
+               return -EINVAL;
+
+       retval = oprofilefs_ulong_from_user(&val, buf, count);
+       if (retval)
+               return retval;
+
+       if (oprofile_hwsampler == val)
+               return -EINVAL;
+
+       retval = oprofile_set_hwsampler(val);
+
+       if (retval)
+               return retval;
+
+       oprofile_hwsampler = val;

arch/s390/oprofile/hwsampler_files.c line 98

Quoted 10 lines
arch/s390/oprofile/hwsampler_files.c new file +10 -0
+       return count;
+}
+
+static const struct file_operations hwsampler_fops = {
+       .read           = hwsampler_read,
+       .write          = hwsampler_write,
+};
+
+int oprofile_create_hwsampling_files(struct super_block *sb,
+                                               struct dentry *root)

This can be made static too.

Quoted 42 lines
arch/s390/oprofile/hwsampler_files.c new file +22 -0
+{
+       struct dentry *hw_dir;
+
+       /* reinitialize default values */
+       oprofile_hwsampler = 1;
+
+       hw_dir = oprofilefs_mkdir(sb, root, "hwsampling");
+       if (!hw_dir)
+               return -EINVAL;
+
+       oprofilefs_create_file(sb, hw_dir, "hwsampler", &hwsampler_fops);
+       oprofilefs_create_ulong(sb, hw_dir, "hw_interval",
+                               &oprofile_hw_interval);
+       oprofilefs_create_ro_ulong(sb, hw_dir, "hw_min_interval",
+                               &oprofile_min_interval);
+       oprofilefs_create_ro_ulong(sb, hw_dir, "hw_max_interval",
+                               &oprofile_max_interval);
+       oprofilefs_create_ulong(sb, hw_dir, "hw_sdbt_blocks",
+                               &oprofile_sdbt_blocks);
+
+       return 0;
+}

Index: linux-2.6/drivers/oprofile/oprof.c
===================================================================

drivers/oprofile/oprof.c +12 -0
@@ -239,10 +239,43 @@ int oprofile_set_ulong(unsigned long *ad
        return err;
 }
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+int oprofile_set_hwsampler(unsigned long val)
+{
+       int err = 0;
+
+       mutex_lock(&start_mutex);
+
+       if (oprofile_started) {
+               err = -EBUSY;
+               goto out;
+       }
+

drivers/oprofile/oprof.c int oprofile_set_ulong(unsigned long *ad · line 263

Quoted 10 lines
drivers/oprofile/oprof.c +10 -0
+       switch (val) {
+       case 1:
+               /* Switch to hardware sampling. */
+               __oprofile_timer_exit();
+               err = oprofile_arch_set_hwsampler(&oprofile_ops);
+               break;
+       case 0:
+               printk(KERN_INFO "oprofile: using timer interrupt.\n");
+               err = __oprofile_timer_init(&oprofile_ops);
+               break;

Is there a use case for switching the mode at runtime? There are kernel parameters to force timer mode while booting or loading the module. I don't like exporting all these timer and hwsampler functions. We could avoid this by making hwsampler architectural code and leaving the timer code as it is.

Quoted 5 lines
drivers/oprofile/oprof.c +5 -0
+       default:
+               err = -EINVAL;
+       }
+
+out:

drivers/oprofile/oprof.c int oprofile_set_ulong(unsigned long *ad · line 278

Quoted 10 lines
drivers/oprofile/oprof.c +6 -0
+       mutex_unlock(&start_mutex);
+       return err;
+}
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
+
 static int __init oprofile_init(void)
 {
        int err;
 
+       memset(&oprofile_ops, 0, sizeof(oprofile_ops));

The struct is already initialized to 0.

drivers/oprofile/oprof.c static int __init oprofile_init(void) · line 288

Quoted 10 lines
drivers/oprofile/oprof.c +3 -0
        err = oprofile_arch_init(&oprofile_ops);
        if (err < 0 || timer) {
                printk(KERN_INFO "oprofile: using timer interrupt.\n");
@@ -250,6 +283,10 @@ static int __init oprofile_init(void)
                if (err)
                        return err;
        }
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+       else if (err == 0)
+               oprofile_arch_set_hwsampler(&oprofile_ops);

I would like to see this in oprofile_arch_init().

Quoted 9 lines
drivers/oprofile/oprof.c +1 -0
+#endif
        return oprofilefs_register();
 }
 

Index: linux-2.6/drivers/oprofile/oprof.h
===================================================================

drivers/oprofile/oprof.h struct dentry; · line 40

Quoted 6 lines
drivers/oprofile/oprof.h +2 -0
@@ -35,7 +35,9 @@ struct dentry;
 
 void oprofile_create_files(struct super_block *sb, struct dentry *root);
 int oprofile_timer_init(struct oprofile_operations *ops);
+int __oprofile_timer_init(struct oprofile_operations *ops);
 void oprofile_timer_exit(void);
+void __oprofile_timer_exit(void);

See my comments above, I don't want to export this.

Quoted 53 lines
drivers/oprofile/oprof.h +0 -0
 int oprofile_set_ulong(unsigned long *addr, unsigned long val);
 int oprofile_set_timeout(unsigned long time);

Index: linux-2.6/drivers/oprofile/timer_int.c
===================================================================

drivers/oprofile/timer_int.c +13 -3
@@ -97,14 +97,13 @@ static struct notifier_block __refdata o
        .notifier_call = oprofile_cpu_notify,
 };
 
-int __init oprofile_timer_init(struct oprofile_operations *ops)
+int  __oprofile_timer_init(struct oprofile_operations *ops)
 {
        int rc;
 
        rc = register_hotcpu_notifier(&oprofile_cpu_notifier);
        if (rc)
                return rc;
-       ops->create_files = NULL;
        ops->setup = NULL;
        ops->shutdown = NULL;
        ops->start = oprofile_hrtimer_start;
@@ -113,7 +112,18 @@ int __init oprofile_timer_init(struct op
        return 0;
 }
 
-void __exit oprofile_timer_exit(void)
+int __init oprofile_timer_init(struct oprofile_operations *ops)
+{
+       return __oprofile_timer_init(ops);
+}
+
+void __oprofile_timer_exit(void)
 {
        unregister_hotcpu_notifier(&oprofile_cpu_notifier);
 }
+
+void __exit oprofile_timer_exit(void)
+{
+       __oprofile_timer_exit();
+}
+

Index: linux-2.6/include/linux/oprofile.h
===================================================================

include/linux/oprofile.h +2 -0
@@ -89,6 +89,21 @@ int oprofile_arch_init(struct oprofile_o
  */
 void oprofile_arch_exit(void);
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+/**

include/linux/oprofile.h int oprofile_arch_init(struct oprofile_o · line 103

Quoted 10 lines
include/linux/oprofile.h +10 -0
+ * setup hardware sampler for oprofiling.
+ */
+
+int oprofile_set_hwsampler(unsigned long);
+
+/**
+ * hardware sampler module initialization for the s390 arch
+ */
+
+int oprofile_arch_set_hwsampler(struct oprofile_operations *ops);

This is not generic code, there is no other architecture that may reuse this. We should move this to arch/s390.

Quoted 14 lines
include/linux/oprofile.h +3 -0
+
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
+
 /**
  * Add a sample. This may be called from any context.
  */

Index: linux-2.6/arch/s390/oprofile/init.c
===================================================================

arch/s390/oprofile/init.c +0 -0
@@ -11,16 +11,51 @@
 #include <linux/oprofile.h>
 #include <linux/init.h>
 #include <linux/errno.h>

arch/s390/oprofile/init.c line 23

Quoted 10 lines
arch/s390/oprofile/init.c +9 -0
+#include <linux/fs.h>
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+#include <asm/hwsampler.h>
+
+extern int oprofile_create_hwsampling_files(struct super_block *sb,
+                                                       struct dentry *root);
+
+extern unsigned long oprofile_min_interval;
+extern unsigned long oprofile_max_interval;

This becomes static if we move it to arch/s390/oprofile/hwsampler.c (see below).

Quoted 18 lines
arch/s390/oprofile/init.c +12 -0
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
 
 extern void s390_backtrace(struct pt_regs * const regs, unsigned int depth);
 
 int __init oprofile_arch_init(struct oprofile_operations* ops)
 {
        ops->backtrace = s390_backtrace;
+
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+       if (hwsampler_setup())
+               return -ENODEV;
+
+       /*
+        * create hwsampler files only if hwsampler_setup() succeeds.
+        */
+       ops->create_files = oprofile_create_hwsampling_files;
+       oprofile_min_interval = hwsampler_query_min_interval();
+       if (oprofile_min_interval < 0) {

arch/s390/oprofile/init.c line 51

Quoted 10 lines
arch/s390/oprofile/init.c +10 -0
+               oprofile_min_interval = 0;
+               return -ENODEV;
+       }
+       oprofile_max_interval = hwsampler_query_max_interval();
+       if (oprofile_max_interval < 0) {
+               oprofile_max_interval = 0;
+               return -ENODEV;
+       }
+       return 0;
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */

Move all the code for CONFIG_OPROFILE_HWSAMPLING_MODE in this file to

arch/s390/oprofile/hwsampler.c

and only export an oprofile_hwsampler_init() function. This can be an empty function stub for the !CONFIG_OPROFILE_HWSAMPLING_MODE case.

arch/s390/oprofile/init.c line 60

Quoted 9 lines
arch/s390/oprofile/init.c +4 -0
+
        return -ENODEV;
 }
 
 void oprofile_arch_exit(void)
 {
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+       hwsampler_shutdown();
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */

Same here...

-Robert

Quoted 3 lines
arch/s390/oprofile/init.c +0 -0
 }
Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#6 lore raw

Re: [patch 2/4] This patch enhances OProfile to support System zs hardware sampling feature

Robert, here is the 2nd part...

Heinz

Quoted 45 lines · Robert Richter
Quoted 37 lines · On 20.12.10 08:05:43, [email protected] wrote:

From: [email protected]

OProfile is enhanced to export all files for controlling System z's hardware sampling, and to invoke hwsampler exported functions to initialize and use System z's hardware sampling.

The patch invokes hwsampler_setup() during oprofile init and exports following hwsampler files under oprofilefs if hwsampler's setup succeeded:

A new directory for hardware sampling based files

/dev/oprofile/hwsampling/

The userland daemon must explicitly write to the following files to disable (or enable) hardware based sampling

/dev/oprofile/hwsampling/hwsampler

to modify the actual sampling rate

/dev/oprofile/hwsampling/hw_interval

to modify the amount of sampling memory (measured in 4K pages)

/dev/oprofile/hwsampling/hw_sdbt_blocks

The following files are read only and show
the possible minimum sampling rate

/dev/oprofile/hwsampling/hw_min_interval

the possible maximum sampling rate

/dev/oprofile/hwsampling/hw_max_interval

The patch splits the oprofile_timer_[init/exit] function so that it can be also called through user context (oprofilefs) to avoid kernel oops.

Quoted 6 lines
---
 arch/s390/oprofile/Makefile          |    1
 arch/s390/oprofile/hwsampler_files.c |  120 +++++++++++++++++++++++++++++++++++
Quoted 4 lines

I would rather see a file hwsampler.c here that contains all oprofile hwsampler code in it and also sets up a struct oprofile_operations* ops.

I added hwsampler.c and also kept the hwsampler_files.c

Quoted 8 lines

Doing so, most of global functions and variables below can be made static.

Quoted 1 line
 arch/s390/oprofile/init.c            |   35 ++++++++++

We should find a better solution than changing all those files only for a single architecture:

Quoted 7 lines
Quoted 4 lines
 drivers/oprofile/oprof.c             |   37 ++++++++++
 drivers/oprofile/oprof.h             |    2
 drivers/oprofile/timer_int.c         |   16 +++-
 include/linux/oprofile.h             |   15 ++++

I want to see most of this in arch/s390.

mostly done, but we need to possibility to switch the modes... see also below

Quoted 33 lines
Quoted 33 lines
 7 files changed, 223 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/s390/oprofile/Makefile
===================================================================

arch/s390/oprofile/Makefile +1 -0
@@ -8,6 +8,7 @@ DRIVER_OBJS = $(addprefix ../../../drive
                timer_int.o )
 
 oprofile-y                             := $(DRIVER_OBJS) init.o backtrace.o
+oprofile-y                             += $(if $(CONFIG_HWSAMPLER), hwsampler_files.o,)
 
 HW_SAMPLER_DRIVER_OBJS = $(addprefix ../../../drivers/s390/hwsampler/, \
                hwsampler-main.o smpctl.o )

Index: linux-2.6/arch/s390/oprofile/hwsampler_files.c
===================================================================

arch/s390/oprofile/hwsampler_files.c new file +14 -0
@@ -0,0 +1,120 @@
+/**
+ * arch/s390/oprofile/hwsampler_files.c
+ *
+ * Copyright IBM Corp. 2010
+ * Author: Mahesh Salgaonkar ([email protected])
+ */
+#include <linux/oprofile.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+
+#include <asm/hwsampler.h>
+
+#define DEFAULT_INTERVAL       4096
+
Quoted 8 lines

arch/s390/oprofile/hwsampler_files.c line 20

Quoted 6 lines
arch/s390/oprofile/hwsampler_files.c new file +6 -0
+#define DEFAULT_SDBT_BLOCKS    1
+#define DEFAULT_SDB_BLOCKS     511
+
+static unsigned long oprofile_hw_interval = DEFAULT_INTERVAL;
+unsigned long oprofile_min_interval;
+unsigned long oprofile_max_interval;

This could be static.

OK, done

Quoted 14 lines

arch/s390/oprofile/hwsampler_files.c line 31

Quoted 12 lines
arch/s390/oprofile/hwsampler_files.c new file +11 -0
+
+static unsigned long oprofile_sdbt_blocks = DEFAULT_SDBT_BLOCKS;
+static unsigned long oprofile_sdb_blocks = DEFAULT_SDB_BLOCKS;
+
+static unsigned long oprofile_hwsampler;
+
+static int oprofile_hwsampler_start(void)
+{
+       int retval;
+
+       printk(KERN_INFO "oprofile_hwsampler_start\n");

This looks like a debug msg.

OK, removed

Quoted 16 lines
Quoted 4 lines
arch/s390/oprofile/hwsampler_files.c new file +3 -0
+
+       retval = hwsampler_allocate(oprofile_sdbt_blocks, oprofile_sdb_blocks);
+       if (retval)

arch/s390/oprofile/hwsampler_files.c line 44

Quoted 10 lines
arch/s390/oprofile/hwsampler_files.c new file +10 -0
+               return retval;
+
+       retval = hwsampler_start_all(oprofile_hw_interval);
+
+       return retval;
+}
+
+static void oprofile_hwsampler_stop(void)
+{
+       printk(KERN_INFO "oprofile_hwsampler_stop\n");

Same here.

OK, removed

Quoted 17 lines

arch/s390/oprofile/hwsampler_files.c line 56

Quoted 13 lines
arch/s390/oprofile/hwsampler_files.c new file +12 -0
+
+       hwsampler_stop_all();
+       hwsampler_deallocate();
+       return;
+}
+
+int oprofile_arch_set_hwsampler(struct oprofile_operations *ops)
+{
+       printk(KERN_INFO "oprofile: using hardware sampling\n");
+       ops->start = oprofile_hwsampler_start;
+       ops->stop = oprofile_hwsampler_stop;
+       ops->cpu_type = "timer";

Wouldn't it be better to have a different cpu_type string here, I don't think the oprofilefs interface is exactly the same as for timer mode. How the daemon distinguishs between both modes?

the user space daemon can live with this unchanged when the kernel is configured for OProfile and CPUMF hardware sampling.

We plan to update the daemon with new options for hardware sampling also and to prepare opreport for appropriate message headers.

Quoted 37 lines
Quoted 37 lines
arch/s390/oprofile/hwsampler_files.c new file +36 -0
+
+       return 0;
+}
+
+static ssize_t hwsampler_read(struct file *file, char __user *buf,
+               size_t count, loff_t *offset)
+{
+       return oprofilefs_ulong_to_user(oprofile_hwsampler, buf, count, offset);
+}
+
+static ssize_t hwsampler_write(struct file *file, char const __user *buf,
+               size_t count, loff_t *offset)
+{
+       unsigned long val;
+       int retval;
+
+       if (*offset)
+               return -EINVAL;
+
+       retval = oprofilefs_ulong_from_user(&val, buf, count);
+       if (retval)
+               return retval;
+
+       if (oprofile_hwsampler == val)
+               return -EINVAL;
+
+       retval = oprofile_set_hwsampler(val);
+
+       if (retval)
+               return retval;
+
+       oprofile_hwsampler = val;
+       return count;
+}
+
+static const struct file_operations hwsampler_fops = {
Quoted 8 lines

arch/s390/oprofile/hwsampler_files.c line 98

Quoted 6 lines
arch/s390/oprofile/hwsampler_files.c new file +6 -0
+       .read           = hwsampler_read,
+       .write          = hwsampler_write,
+};
+
+int oprofile_create_hwsampling_files(struct super_block *sb,
+                                               struct dentry *root)

This can be made static too.

OK, done

Quoted 51 lines
Quoted 51 lines
arch/s390/oprofile/hwsampler_files.c new file +22 -0
+{
+       struct dentry *hw_dir;
+
+       /* reinitialize default values */
+       oprofile_hwsampler = 1;
+
+       hw_dir = oprofilefs_mkdir(sb, root, "hwsampling");
+       if (!hw_dir)
+               return -EINVAL;
+
+       oprofilefs_create_file(sb, hw_dir, "hwsampler", &hwsampler_fops);
+       oprofilefs_create_ulong(sb, hw_dir, "hw_interval",
+                               &oprofile_hw_interval);
+       oprofilefs_create_ro_ulong(sb, hw_dir, "hw_min_interval",
+                               &oprofile_min_interval);
+       oprofilefs_create_ro_ulong(sb, hw_dir, "hw_max_interval",
+                               &oprofile_max_interval);
+       oprofilefs_create_ulong(sb, hw_dir, "hw_sdbt_blocks",
+                               &oprofile_sdbt_blocks);
+
+       return 0;
+}

Index: linux-2.6/drivers/oprofile/oprof.c
===================================================================

drivers/oprofile/oprof.c +20 -0
@@ -239,10 +239,43 @@ int oprofile_set_ulong(unsigned long *ad
        return err;
 }
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+int oprofile_set_hwsampler(unsigned long val)
+{
+       int err = 0;
+
+       mutex_lock(&start_mutex);
+
+       if (oprofile_started) {
+               err = -EBUSY;
+               goto out;
+       }
+
+       switch (val) {
+       case 1:
+               /* Switch to hardware sampling. */
+               __oprofile_timer_exit();
+               err = oprofile_arch_set_hwsampler(&oprofile_ops);
+               break;
+       case 0:
+               printk(KERN_INFO "oprofile: using timer interrupt.\n");
Quoted 8 lines

drivers/oprofile/oprof.c int oprofile_set_ulong(unsigned long *ad · line 263

Quoted 2 lines
drivers/oprofile/oprof.c +2 -0
+               err = __oprofile_timer_init(&oprofile_ops);
+               break;

Is there a use case for switching the mode at runtime? There are kernel parameters to force timer mode while booting or loading the module. I don't like exporting all these timer and hwsampler functions. We could avoid this by making hwsampler architectural code and leaving the timer code as it is.

hardware sampling might have a need for lots of kernel memory, which might not be available in all circumstances. It was nice if one could switch to timer based sampling in such a case.

Quoted 15 lines
Quoted 15 lines
drivers/oprofile/oprof.c +10 -0
+       default:
+               err = -EINVAL;
+       }
+
+out:
+       mutex_unlock(&start_mutex);
+       return err;
+}
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
+
 static int __init oprofile_init(void)
 {
        int err;
 
Quoted 3 lines

drivers/oprofile/oprof.c int oprofile_set_ulong(unsigned long *ad · line 278

Quoted 1 line
drivers/oprofile/oprof.c +1 -0
+       memset(&oprofile_ops, 0, sizeof(oprofile_ops));

The struct is already initialized to 0.

OK, statement removed

Quoted 14 lines

drivers/oprofile/oprof.c static int __init oprofile_init(void) · line 288

Quoted 11 lines
drivers/oprofile/oprof.c +3 -0
        err = oprofile_arch_init(&oprofile_ops);
        if (err < 0 || timer) {
                printk(KERN_INFO "oprofile: using timer interrupt.\n");
@@ -250,6 +283,10 @@ static int __init oprofile_init(void)
                if (err)
                        return err;
        }
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+       else if (err == 0)
+               oprofile_arch_set_hwsampler(&oprofile_ops);

I would like to see this in oprofile_arch_init().

OK, done

Quoted 71 lines
Quoted 11 lines
drivers/oprofile/oprof.c +1 -0
+#endif
        return oprofilefs_register();
 }
 

Index: linux-2.6/drivers/oprofile/oprof.h
===================================================================

drivers/oprofile/oprof.h struct dentry; · line 40

Quoted 6 lines
drivers/oprofile/oprof.h +2 -0
@@ -35,7 +35,9 @@ struct dentry;
 
 void oprofile_create_files(struct super_block *sb, struct dentry *root);
 int oprofile_timer_init(struct oprofile_operations *ops);
+int __oprofile_timer_init(struct oprofile_operations *ops);
 void oprofile_timer_exit(void);
+void __oprofile_timer_exit(void);

See my comments above, I don't want to export this.

Quoted 51 lines
drivers/oprofile/oprof.h +0 -0
 int oprofile_set_ulong(unsigned long *addr, unsigned long val);
 int oprofile_set_timeout(unsigned long time);

Index: linux-2.6/drivers/oprofile/timer_int.c
===================================================================

drivers/oprofile/timer_int.c +13 -3
@@ -97,14 +97,13 @@ static struct notifier_block __refdata o
        .notifier_call = oprofile_cpu_notify,
 };
 
-int __init oprofile_timer_init(struct oprofile_operations *ops)
+int  __oprofile_timer_init(struct oprofile_operations *ops)
 {
        int rc;
 
        rc = register_hotcpu_notifier(&oprofile_cpu_notifier);
        if (rc)
                return rc;
-       ops->create_files = NULL;
        ops->setup = NULL;
        ops->shutdown = NULL;
        ops->start = oprofile_hrtimer_start;
@@ -113,7 +112,18 @@ int __init oprofile_timer_init(struct op
        return 0;
 }
 
-void __exit oprofile_timer_exit(void)
+int __init oprofile_timer_init(struct oprofile_operations *ops)
+{
+       return __oprofile_timer_init(ops);
+}
+
+void __oprofile_timer_exit(void)
 {
        unregister_hotcpu_notifier(&oprofile_cpu_notifier);
 }
+
+void __exit oprofile_timer_exit(void)
+{
+       __oprofile_timer_exit();
+}
+

Index: linux-2.6/include/linux/oprofile.h
===================================================================

include/linux/oprofile.h +0 -0
@@ -89,6 +89,21 @@ int oprofile_arch_init(struct oprofile_o
  */
 void oprofile_arch_exit(void);
 
Quoted 16 lines

include/linux/oprofile.h int oprofile_arch_init(struct oprofile_o · line 103

Quoted 12 lines
include/linux/oprofile.h +12 -0
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+/**
+ * setup hardware sampler for oprofiling.
+ */
+
+int oprofile_set_hwsampler(unsigned long);
+
+/**
+ * hardware sampler module initialization for the s390 arch
+ */
+
+int oprofile_arch_set_hwsampler(struct oprofile_operations *ops);

This is not generic code, there is no other architecture that may reuse this. We should move this to arch/s390.

Hmm, we need it to be able to switch modes.

Quoted 16 lines
Quoted 16 lines
include/linux/oprofile.h +3 -0
+
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
+
 /**
  * Add a sample. This may be called from any context.
  */

Index: linux-2.6/arch/s390/oprofile/init.c
===================================================================

arch/s390/oprofile/init.c +1 -0
@@ -11,16 +11,51 @@
 #include <linux/oprofile.h>
 #include <linux/init.h>
 #include <linux/errno.h>
+#include <linux/fs.h>
 
Quoted 11 lines

arch/s390/oprofile/init.c line 23

Quoted 8 lines
arch/s390/oprofile/init.c +8 -0
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+#include <asm/hwsampler.h>
+
+extern int oprofile_create_hwsampling_files(struct super_block *sb,
+                                                       struct dentry *root);
+
+extern unsigned long oprofile_min_interval;
+extern unsigned long oprofile_max_interval;

This becomes static if we move it to arch/s390/oprofile/hwsampler.c (see below).

done

Quoted 32 lines
Quoted 19 lines
arch/s390/oprofile/init.c +12 -0
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */
 
 extern void s390_backtrace(struct pt_regs * const regs, unsigned int depth);
 
 int __init oprofile_arch_init(struct oprofile_operations* ops)
 {
        ops->backtrace = s390_backtrace;
+
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+       if (hwsampler_setup())
+               return -ENODEV;
+
+       /*
+        * create hwsampler files only if hwsampler_setup() succeeds.
+        */
+       ops->create_files = oprofile_create_hwsampling_files;
+       oprofile_min_interval = hwsampler_query_min_interval();
+       if (oprofile_min_interval < 0) {

arch/s390/oprofile/init.c line 51

Quoted 10 lines
arch/s390/oprofile/init.c +10 -0
+               oprofile_min_interval = 0;
+               return -ENODEV;
+       }
+       oprofile_max_interval = hwsampler_query_max_interval();
+       if (oprofile_max_interval < 0) {
+               oprofile_max_interval = 0;
+               return -ENODEV;
+       }
+       return 0;
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */

Move all the code for CONFIG_OPROFILE_HWSAMPLING_MODE in this file to

Quoted 5 lines

arch/s390/oprofile/hwsampler.c

and only export an oprofile_hwsampler_init() function. This can be an empty function stub for the !CONFIG_OPROFILE_HWSAMPLING_MODE case.

done

Quoted 11 lines

arch/s390/oprofile/init.c line 60

Quoted 9 lines
arch/s390/oprofile/init.c +4 -0
+
        return -ENODEV;
 }
 
 void oprofile_arch_exit(void)
 {
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+       hwsampler_shutdown();
+#endif /* CONFIG_OPROFILE_HWSAMPLING_MODE */

Same here...

done

Quoted 7 lines

-Robert

Quoted 3 lines
arch/s390/oprofile/init.c +0 -0
 }
#7 lore raw

PATCH 3/4 This patch introduces a new oprofile sample add function (oprofile_add_ext_hw_sample)

In reply to [email protected]

From: [email protected]

This patch introduces a new oprofile sample add function (oprofile_add_ext_hw_sample) that can also take task_struct as an argument, which is used by the hwsampler kernel module when copying hardware samples to OProfile buffers.

---
 drivers/oprofile/cpu_buffer.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)
drivers/oprofile/cpu_buffer.c +19 -7
Index: linux-2.6/drivers/oprofile/cpu_buffer.c
===================================================================
@@ -22,6 +22,7 @@
 #include <linux/sched.h>
 #include <linux/oprofile.h>
 #include <linux/errno.h>
+#include <linux/module.h>
 
 #include "event_buffer.h"
 #include "cpu_buffer.h"
@@ -258,8 +259,10 @@ op_add_sample(struct oprofile_cpu_buffer
  */
 static int
 log_sample(struct oprofile_cpu_buffer *cpu_buf, unsigned long pc,
-	   unsigned long backtrace, int is_kernel, unsigned long event)
+	unsigned long backtrace, int is_kernel, unsigned long event,
+	struct task_struct *task)
 {
+	struct task_struct *tsk = task ? task : current;
 	cpu_buf->sample_received++;
 
 	if (pc == ESCAPE_CODE) {
@@ -267,7 +270,7 @@ log_sample(struct oprofile_cpu_buffer *c
 		return 0;
 	}
 
-	if (op_add_code(cpu_buf, backtrace, is_kernel, current))
+	if (op_add_code(cpu_buf, backtrace, is_kernel, tsk))
 		goto fail;
 
 	if (op_add_sample(cpu_buf, pc, event))
@@ -292,7 +295,8 @@ static inline void oprofile_end_trace(st
 
 static inline void
 __oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
-			  unsigned long event, int is_kernel)
+			  unsigned long event, int is_kernel,
+			  struct task_struct *task)
 {
 	struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(op_cpu_buffer);
 	unsigned long backtrace = oprofile_backtrace_depth;
@@ -301,7 +305,7 @@ __oprofile_add_ext_sample(unsigned long
 	 * if log_sample() fail we can't backtrace since we lost the
 	 * source of this event
 	 */
-	if (!log_sample(cpu_buf, pc, backtrace, is_kernel, event))
+	if (!log_sample(cpu_buf, pc, backtrace, is_kernel, event, task))
 		/* failed */
 		return;
 
@@ -316,9 +320,17 @@ __oprofile_add_ext_sample(unsigned long
 void oprofile_add_ext_sample(unsigned long pc, struct pt_regs * const regs,
 			     unsigned long event, int is_kernel)
 {
-	__oprofile_add_ext_sample(pc, regs, event, is_kernel);
+	__oprofile_add_ext_sample(pc, regs, event, is_kernel, NULL);
 }
 
+void oprofile_add_ext_hw_sample(unsigned long pc, struct pt_regs * const regs,
+				unsigned long event, int is_kernel,
+				struct task_struct *task)
+{
+	__oprofile_add_ext_sample(pc, regs, event, is_kernel, task);
+}
+EXPORT_SYMBOL_GPL(oprofile_add_ext_hw_sample);
+
 void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
 {
 	int is_kernel;
@@ -332,7 +344,7 @@ void oprofile_add_sample(struct pt_regs
 		pc = ESCAPE_CODE; /* as this causes an early return. */
 	}
 
-	__oprofile_add_ext_sample(pc, regs, event, is_kernel);
+	__oprofile_add_ext_sample(pc, regs, event, is_kernel, NULL);
 }
 
 /*
@@ -403,7 +415,7 @@ int oprofile_write_commit(struct op_entr
 void oprofile_add_pc(unsigned long pc, int is_kernel, unsigned long event)
 {
 	struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(op_cpu_buffer);
-	log_sample(cpu_buf, pc, 0, is_kernel, event);
+	log_sample(cpu_buf, pc, 0, is_kernel, event, NULL);
 }
 
 void oprofile_add_trace(unsigned long pc)
#8 lore raw

Re: [patch 3/4] This patch introduces a new oprofile sample add function (oprofile_add_ext_hw_sample)

Quoted 13 lines · On 20.12.10 08:05:44, [email protected] wrote:

From: [email protected]

This patch introduces a new oprofile sample add function (oprofile_add_ext_hw_sample) that can also take task_struct as an argument, which is used by the hwsampler kernel module when copying hardware samples to OProfile buffers.

---
 drivers/oprofile/cpu_buffer.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

drivers/oprofile/cpu_buffer.c __oprofile_add_ext_sample(unsigned long · line 332

Quoted 7 lines
drivers/oprofile/cpu_buffer.c +7 -0
+void oprofile_add_ext_hw_sample(unsigned long pc, struct pt_regs * const regs,
+				unsigned long event, int is_kernel,
+				struct task_struct *task)
+{
+	__oprofile_add_ext_sample(pc, regs, event, is_kernel, task);
+}
+EXPORT_SYMBOL_GPL(oprofile_add_ext_hw_sample);

Hmm, I am not convinced of this new interface between the hwsampler module and oprofile. It is asymmetric and bloats the function's parameters list. A first simplification would be to not implement hwsampler as module and integrate this in oprofile. Then, we can look for a better way to add samples to the oprofile buffer. What do you think?

-Robert

Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#9 lore raw

Re: [patch 3/4] This patch introduces a new oprofile sample add function (oprofile_add_ext_hw_sample)

Quoted 29 lines · Robert Richter
Quoted 13 lines · On 20.12.10 08:05:44, [email protected] wrote:

From: [email protected]

This patch introduces a new oprofile sample add function (oprofile_add_ext_hw_sample) that can also take task_struct as an argument, which is used by the hwsampler kernel module when copying hardware samples to OProfile buffers.

---
 drivers/oprofile/cpu_buffer.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

drivers/oprofile/cpu_buffer.c __oprofile_add_ext_sample(unsigned long · line 332

Quoted 7 lines
drivers/oprofile/cpu_buffer.c +7 -0
+void oprofile_add_ext_hw_sample(unsigned long pc, struct pt_regs * const regs,
+				unsigned long event, int is_kernel,
+				struct task_struct *task)
+{
+	__oprofile_add_ext_sample(pc, regs, event, is_kernel, task);
+}
+EXPORT_SYMBOL_GPL(oprofile_add_ext_hw_sample);

Hmm, I am not convinced of this new interface between the hwsampler module and oprofile. It is asymmetric and bloats the function's parameters list. A first simplification would be to not implement hwsampler as module and integrate this in oprofile. Then, we can look for a better way to add samples to the oprofile buffer. What do you think?

Quoted 2 lines

-Robert

Robert, thanks for all the comments. I need to think and discuss this.

-Heinz

#10 lore raw

Re: [patch 3/4] This patch introduces a new oprofile sample add function (oprofile_add_ext_hw_sample)

In reply to Robert Richter

Robert,

here is the 3rd part.

Thanks a lot for your comments!

Heinz

Quoted 14 lines · Robert Richter
Quoted 12 lines · On 20.12.10 08:05:44, [email protected] wrote:

From: [email protected]

This patch introduces a new oprofile sample add function (oprofile_add_ext_hw_sample) that can also take task_struct as an argument, which is used by the hwsampler kernel module when copying hardware samples to OProfile buffers.

---
 drivers/oprofile/cpu_buffer.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)
Quoted 15 lines

drivers/oprofile/cpu_buffer.c __oprofile_add_ext_sample(unsigned long · line 332

Quoted 7 lines
drivers/oprofile/cpu_buffer.c +7 -0
+void oprofile_add_ext_hw_sample(unsigned long pc, struct pt_regs * const regs,
+				unsigned long event, int is_kernel,
+				struct task_struct *task)
+{
+	__oprofile_add_ext_sample(pc, regs, event, is_kernel, task);
+}
+EXPORT_SYMBOL_GPL(oprofile_add_ext_hw_sample);

Hmm, I am not convinced of this new interface between the hwsampler module and oprofile. It is asymmetric and bloats the function's parameters list. A first simplification would be to not implement hwsampler as module and integrate this in oprofile. Then, we can look for a better way to add samples to the oprofile buffer. What do you think?

ok, as you suggested hwsampler is now integrated in oprofile, and the EXPORT is gone, instead I added it to oprofile.h, which I fear you will not accept but I try.

Quoted 2 lines

-Robert

#11 lore raw

PATCH 4/4 Handle memory unmap while hardware sampling is running

In reply to [email protected]

From: [email protected]

During memory unmap hardware sampling is deactivated.
After all samples have been collected hardware sampling is reactivated again.

---
 arch/s390/oprofile/hwsampler_files.c |    2 +-
 drivers/oprofile/buffer_sync.c       |   13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
drivers/oprofile/buffer_sync.c +13 -0
Index: linux-2.6/drivers/oprofile/buffer_sync.c
===================================================================
@@ -32,6 +32,11 @@
 #include <linux/sched.h>
 #include <linux/gfp.h>
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+#include <asm/hwsampler.h>
+extern unsigned long oprofile_hwsampler;
+#endif
+
 #include "oprofile_stats.h"
 #include "event_buffer.h"
 #include "cpu_buffer.h"
@@ -513,6 +518,10 @@ void sync_buffer(int cpu)
 
 	mutex_lock(&buffer_mutex);
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+	if (oprofile_hwsampler)
+		hwsampler_deactivate(cpu);
+#endif
 	add_cpu_switch(cpu);
 
 	op_cpu_buffer_reset(cpu);
@@ -569,6 +578,10 @@ void sync_buffer(int cpu)
 
 	mark_done(cpu);
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+	if (oprofile_hwsampler)
+		hwsampler_activate(cpu);
+#endif
 	mutex_unlock(&buffer_mutex);
 }
 
arch/s390/oprofile/hwsampler_files.c +1 -1
Index: linux-2.6/arch/s390/oprofile/hwsampler_files.c
===================================================================
@@ -22,7 +22,7 @@ unsigned long oprofile_max_interval;
 static unsigned long oprofile_sdbt_blocks = DEFAULT_SDBT_BLOCKS;
 static unsigned long oprofile_sdb_blocks = DEFAULT_SDB_BLOCKS;
 
-static unsigned long oprofile_hwsampler;
+unsigned long oprofile_hwsampler;
 
 static int oprofile_hwsampler_start(void)
 {
#12 lore raw

Re: [patch 4/4] Handle memory unmap while hardware sampling is running

Quoted 4 lines · On 20.12.10 08:05:45, [email protected] wrote:

From: [email protected]

During memory unmap hardware sampling is deactivated.
After all samples have been collected hardware sampling is reactivated again.

Do you mean you switch off the profiler for each buffer sync? This happens per default every 100ms. Why is this needed? Which memory is unmapped?

This hooks in the sync_buffer() are ugly. We must try to find a better solution here.

There is oprofile_put_buff() that allows to write directly to the the event buffer which is used in powerpc/cell.

-Robert

Quoted 62 lines
---
 arch/s390/oprofile/hwsampler_files.c |    2 +-
 drivers/oprofile/buffer_sync.c       |   13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/oprofile/buffer_sync.c
===================================================================

drivers/oprofile/buffer_sync.c +13 -0
@@ -32,6 +32,11 @@
 #include <linux/sched.h>
 #include <linux/gfp.h>
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+#include <asm/hwsampler.h>
+extern unsigned long oprofile_hwsampler;
+#endif
+
 #include "oprofile_stats.h"
 #include "event_buffer.h"
 #include "cpu_buffer.h"
@@ -513,6 +518,10 @@ void sync_buffer(int cpu)
 
 	mutex_lock(&buffer_mutex);
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+	if (oprofile_hwsampler)
+		hwsampler_deactivate(cpu);
+#endif
 	add_cpu_switch(cpu);
 
 	op_cpu_buffer_reset(cpu);
@@ -569,6 +578,10 @@ void sync_buffer(int cpu)
 
 	mark_done(cpu);
 
+#ifdef CONFIG_OPROFILE_HWSAMPLING_MODE
+	if (oprofile_hwsampler)
+		hwsampler_activate(cpu);
+#endif
 	mutex_unlock(&buffer_mutex);
 }
 

Index: linux-2.6/arch/s390/oprofile/hwsampler_files.c
===================================================================

arch/s390/oprofile/hwsampler_files.c +1 -1
@@ -22,7 +22,7 @@ unsigned long oprofile_max_interval;
 static unsigned long oprofile_sdbt_blocks = DEFAULT_SDBT_BLOCKS;
 static unsigned long oprofile_sdb_blocks = DEFAULT_SDB_BLOCKS;
 
-static unsigned long oprofile_hwsampler;
+unsigned long oprofile_hwsampler;
 
 static int oprofile_hwsampler_start(void)
 {
Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center