Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Hendrik Brueckner]

On Tue, Jun 14, 2016 at 05:56:09PM +0200, Heiko Carstens wrote:
> On Tue, Jun 14, 2016 at 03:28:23PM +0200, Robert Richter wrote:
> > > 
> > >  Documentation/kernel-parameters.txt |2 -
> > >  arch/s390/oprofile/Makefile |1 -
> > >  arch/s390/oprofile/hwsampler.c  | 1178 
> > > 
> > >  arch/s390/oprofile/hwsampler.h  |   63 --
> > >  arch/s390/oprofile/init.c   |  489 -
> > >  arch/s390/oprofile/op_counter.h |   21 -
> > >  6 files changed, 1754 deletions(-)
> > 
> > Could you send the patch for review anyway?
> 
> Yes, of course. I added it below, hope that's ok.
> [I did not yet add your Ack]
> 
> From 74ab0aefe07573358cc6a0c462ddc62ccba4c0cf Mon Sep 17 00:00:00 2001
> From: Heiko Carstens 
> Date: Fri, 10 Jun 2016 10:40:06 +0200
> Subject: [PATCH] s390/oprofile: remove hardware sampler support
> 
> Remove hardware sampler support from oprofile module.
> 
> The oprofile user space utilty has been switched to use the kernel
> perf interface, for which we also provide hardware sampling support.
> 
> In addition the hardware sampling support is also slightly broken: it
> supports only 16 bits for the pid and therefore would generate wrong
> results on machines which have a pid >64k.
> 
> Also the pt_regs structure which was passed to oprofile common code
> cannot necessarily be used to generate sane backtraces, since the
> task(s) in question may run while the samples are fed to oprofile.
> So the result would be more or less random.
> 
> However given that the only user space tools switched to the perf
> interface already four years ago the hardware sampler code seems to be
> unused code, and therefore it should be reasonable to remove it.
> 
> The timer based oprofile support continues to work.
> 
> Signed-off-by: Heiko Carstens 
> ---
>  Documentation/kernel-parameters.txt |2 -
>  arch/s390/oprofile/Makefile |1 -
>  arch/s390/oprofile/hwsampler.c  | 1178 
> ---
>  arch/s390/oprofile/hwsampler.h  |   63 --
>  arch/s390/oprofile/init.c   |  489 ---
>  arch/s390/oprofile/op_counter.h |   21 -
>  6 files changed, 1754 deletions(-)
>  delete mode 100644 arch/s390/oprofile/hwsampler.c
>  delete mode 100644 arch/s390/oprofile/hwsampler.h
>  delete mode 100644 arch/s390/oprofile/op_counter.h
> 

Reviewed-by: Hendrik Brueckner 

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Hendrik Brueckner]

On Tue, Jun 14, 2016 at 05:56:09PM +0200, Heiko Carstens wrote:
> On Tue, Jun 14, 2016 at 03:28:23PM +0200, Robert Richter wrote:
> > > 
> > >  Documentation/kernel-parameters.txt |2 -
> > >  arch/s390/oprofile/Makefile |1 -
> > >  arch/s390/oprofile/hwsampler.c  | 1178 
> > > 
> > >  arch/s390/oprofile/hwsampler.h  |   63 --
> > >  arch/s390/oprofile/init.c   |  489 -
> > >  arch/s390/oprofile/op_counter.h |   21 -
> > >  6 files changed, 1754 deletions(-)
> > 
> > Could you send the patch for review anyway?
> 
> Yes, of course. I added it below, hope that's ok.
> [I did not yet add your Ack]
> 
> From 74ab0aefe07573358cc6a0c462ddc62ccba4c0cf Mon Sep 17 00:00:00 2001
> From: Heiko Carstens 
> Date: Fri, 10 Jun 2016 10:40:06 +0200
> Subject: [PATCH] s390/oprofile: remove hardware sampler support
> 
> Remove hardware sampler support from oprofile module.
> 
> The oprofile user space utilty has been switched to use the kernel
> perf interface, for which we also provide hardware sampling support.
> 
> In addition the hardware sampling support is also slightly broken: it
> supports only 16 bits for the pid and therefore would generate wrong
> results on machines which have a pid >64k.
> 
> Also the pt_regs structure which was passed to oprofile common code
> cannot necessarily be used to generate sane backtraces, since the
> task(s) in question may run while the samples are fed to oprofile.
> So the result would be more or less random.
> 
> However given that the only user space tools switched to the perf
> interface already four years ago the hardware sampler code seems to be
> unused code, and therefore it should be reasonable to remove it.
> 
> The timer based oprofile support continues to work.
> 
> Signed-off-by: Heiko Carstens 
> ---
>  Documentation/kernel-parameters.txt |2 -
>  arch/s390/oprofile/Makefile |1 -
>  arch/s390/oprofile/hwsampler.c  | 1178 
> ---
>  arch/s390/oprofile/hwsampler.h  |   63 --
>  arch/s390/oprofile/init.c   |  489 ---
>  arch/s390/oprofile/op_counter.h |   21 -
>  6 files changed, 1754 deletions(-)
>  delete mode 100644 arch/s390/oprofile/hwsampler.c
>  delete mode 100644 arch/s390/oprofile/hwsampler.h
>  delete mode 100644 arch/s390/oprofile/op_counter.h
> 

Reviewed-by: Hendrik Brueckner 

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Heiko Carstens]

On Tue, Jun 14, 2016 at 03:28:23PM +0200, Robert Richter wrote:
> > 
> >  Documentation/kernel-parameters.txt |2 -
> >  arch/s390/oprofile/Makefile |1 -
> >  arch/s390/oprofile/hwsampler.c  | 1178 
> >  arch/s390/oprofile/hwsampler.h  |   63 --
> >  arch/s390/oprofile/init.c   |  489 -
> >  arch/s390/oprofile/op_counter.h |   21 -
> >  6 files changed, 1754 deletions(-)
> 
> Could you send the patch for review anyway?

Yes, of course. I added it below, hope that's ok.
[I did not yet add your Ack]

>From 74ab0aefe07573358cc6a0c462ddc62ccba4c0cf Mon Sep 17 00:00:00 2001
From: Heiko Carstens 
Date: Fri, 10 Jun 2016 10:40:06 +0200
Subject: [PATCH] s390/oprofile: remove hardware sampler support

Remove hardware sampler support from oprofile module.

The oprofile user space utilty has been switched to use the kernel
perf interface, for which we also provide hardware sampling support.

In addition the hardware sampling support is also slightly broken: it
supports only 16 bits for the pid and therefore would generate wrong
results on machines which have a pid >64k.

Also the pt_regs structure which was passed to oprofile common code
cannot necessarily be used to generate sane backtraces, since the
task(s) in question may run while the samples are fed to oprofile.
So the result would be more or less random.

However given that the only user space tools switched to the perf
interface already four years ago the hardware sampler code seems to be
unused code, and therefore it should be reasonable to remove it.

The timer based oprofile support continues to work.

Signed-off-by: Heiko Carstens 
---
 Documentation/kernel-parameters.txt |2 -
 arch/s390/oprofile/Makefile |1 -
 arch/s390/oprofile/hwsampler.c  | 1178 ---
 arch/s390/oprofile/hwsampler.h  |   63 --
 arch/s390/oprofile/init.c   |  489 ---
 arch/s390/oprofile/op_counter.h |   21 -
 6 files changed, 1754 deletions(-)
 delete mode 100644 arch/s390/oprofile/hwsampler.c
 delete mode 100644 arch/s390/oprofile/hwsampler.h
 delete mode 100644 arch/s390/oprofile/op_counter.h

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 82b42c958d1c..68b0c022861a 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2788,8 +2788,6 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
timer: [X86] Force use of architectural NMI
timer mode (see also oprofile.timer
for generic hr timer mode)
-   [s390] Force legacy basic mode sampling
-(report cpu_type "timer")
 
oops=panic  Always panic on oopses. Default is to just kill the
process, but there is a small probability of
diff --git a/arch/s390/oprofile/Makefile b/arch/s390/oprofile/Makefile
index 496e4a7ee00e..e9dd41b0b8d3 100644
--- a/arch/s390/oprofile/Makefile
+++ b/arch/s390/oprofile/Makefile
@@ -7,4 +7,3 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
timer_int.o )
 
 oprofile-y :=  $(DRIVER_OBJS) init.o
-oprofile-y +=  hwsampler.o
diff --git a/arch/s390/oprofile/hwsampler.c b/arch/s390/oprofile/hwsampler.c
deleted file mode 100644
index ff9b4eb34589..
--- a/arch/s390/oprofile/hwsampler.c
+++ /dev/null
@@ -1,1178 +0,0 @@
-/*
- * Copyright IBM Corp. 2010
- * Author: Heinz Graalfs 
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-#include 
-#include 
-
-#include "hwsampler.h"
-#include "op_counter.h"
-
-#define MAX_NUM_SDB 511
-#define MIN_NUM_SDB 1
-
-DECLARE_PER_CPU(struct hws_cpu_buffer, sampler_cpu_buffer);
-
-struct hws_execute_parms {
-   void *buffer;
-   signed int rc;
-};
-
-DEFINE_PER_CPU(struct hws_cpu_buffer, sampler_cpu_buffer);
-EXPORT_PER_CPU_SYMBOL(sampler_cpu_buffer);
-
-static DEFINE_MUTEX(hws_sem);
-static DEFINE_MUTEX(hws_sem_oom);
-
-static unsigned char hws_flush_all;
-static unsigned int hws_oom;
-static unsigned int hws_alert;
-static struct workqueue_struct *hws_wq;
-
-static unsigned int hws_state;
-enum {
-   HWS_INIT = 1,
-   HWS_DEALLOCATED,
-   HWS_STOPPED,
-   HWS_STARTED,
-   HWS_STOPPING };
-
-/* set to 1 if called by kernel during memory allocation */
-static unsigned char oom_killer_was_active;
-/* size of SDBT and SDB as of allocate API */
-static unsigned long num_sdbt = 100;
-static unsigned long num_sdb = 511;
-/* sampling interval (machine cycles) */
-static unsigned long interval;
-
-static unsigned long min_sampler_rate;
-static unsigned long max_sampler_rate;
-
-static void execute_qsi(void *parms)
-{
-  

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Heiko Carstens]

On Tue, Jun 14, 2016 at 03:28:23PM +0200, Robert Richter wrote:
> > 
> >  Documentation/kernel-parameters.txt |2 -
> >  arch/s390/oprofile/Makefile |1 -
> >  arch/s390/oprofile/hwsampler.c  | 1178 
> >  arch/s390/oprofile/hwsampler.h  |   63 --
> >  arch/s390/oprofile/init.c   |  489 -
> >  arch/s390/oprofile/op_counter.h |   21 -
> >  6 files changed, 1754 deletions(-)
> 
> Could you send the patch for review anyway?

Yes, of course. I added it below, hope that's ok.
[I did not yet add your Ack]

>From 74ab0aefe07573358cc6a0c462ddc62ccba4c0cf Mon Sep 17 00:00:00 2001
From: Heiko Carstens 
Date: Fri, 10 Jun 2016 10:40:06 +0200
Subject: [PATCH] s390/oprofile: remove hardware sampler support

Remove hardware sampler support from oprofile module.

The oprofile user space utilty has been switched to use the kernel
perf interface, for which we also provide hardware sampling support.

In addition the hardware sampling support is also slightly broken: it
supports only 16 bits for the pid and therefore would generate wrong
results on machines which have a pid >64k.

Also the pt_regs structure which was passed to oprofile common code
cannot necessarily be used to generate sane backtraces, since the
task(s) in question may run while the samples are fed to oprofile.
So the result would be more or less random.

However given that the only user space tools switched to the perf
interface already four years ago the hardware sampler code seems to be
unused code, and therefore it should be reasonable to remove it.

The timer based oprofile support continues to work.

Signed-off-by: Heiko Carstens 
---
 Documentation/kernel-parameters.txt |2 -
 arch/s390/oprofile/Makefile |1 -
 arch/s390/oprofile/hwsampler.c  | 1178 ---
 arch/s390/oprofile/hwsampler.h  |   63 --
 arch/s390/oprofile/init.c   |  489 ---
 arch/s390/oprofile/op_counter.h |   21 -
 6 files changed, 1754 deletions(-)
 delete mode 100644 arch/s390/oprofile/hwsampler.c
 delete mode 100644 arch/s390/oprofile/hwsampler.h
 delete mode 100644 arch/s390/oprofile/op_counter.h

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 82b42c958d1c..68b0c022861a 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2788,8 +2788,6 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
timer: [X86] Force use of architectural NMI
timer mode (see also oprofile.timer
for generic hr timer mode)
-   [s390] Force legacy basic mode sampling
-(report cpu_type "timer")
 
oops=panic  Always panic on oopses. Default is to just kill the
process, but there is a small probability of
diff --git a/arch/s390/oprofile/Makefile b/arch/s390/oprofile/Makefile
index 496e4a7ee00e..e9dd41b0b8d3 100644
--- a/arch/s390/oprofile/Makefile
+++ b/arch/s390/oprofile/Makefile
@@ -7,4 +7,3 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
timer_int.o )
 
 oprofile-y :=  $(DRIVER_OBJS) init.o
-oprofile-y +=  hwsampler.o
diff --git a/arch/s390/oprofile/hwsampler.c b/arch/s390/oprofile/hwsampler.c
deleted file mode 100644
index ff9b4eb34589..
--- a/arch/s390/oprofile/hwsampler.c
+++ /dev/null
@@ -1,1178 +0,0 @@
-/*
- * Copyright IBM Corp. 2010
- * Author: Heinz Graalfs 
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-#include 
-#include 
-
-#include "hwsampler.h"
-#include "op_counter.h"
-
-#define MAX_NUM_SDB 511
-#define MIN_NUM_SDB 1
-
-DECLARE_PER_CPU(struct hws_cpu_buffer, sampler_cpu_buffer);
-
-struct hws_execute_parms {
-   void *buffer;
-   signed int rc;
-};
-
-DEFINE_PER_CPU(struct hws_cpu_buffer, sampler_cpu_buffer);
-EXPORT_PER_CPU_SYMBOL(sampler_cpu_buffer);
-
-static DEFINE_MUTEX(hws_sem);
-static DEFINE_MUTEX(hws_sem_oom);
-
-static unsigned char hws_flush_all;
-static unsigned int hws_oom;
-static unsigned int hws_alert;
-static struct workqueue_struct *hws_wq;
-
-static unsigned int hws_state;
-enum {
-   HWS_INIT = 1,
-   HWS_DEALLOCATED,
-   HWS_STOPPED,
-   HWS_STARTED,
-   HWS_STOPPING };
-
-/* set to 1 if called by kernel during memory allocation */
-static unsigned char oom_killer_was_active;
-/* size of SDBT and SDB as of allocate API */
-static unsigned long num_sdbt = 100;
-static unsigned long num_sdb = 511;
-/* sampling interval (machine cycles) */
-static unsigned long interval;
-
-static unsigned long min_sampler_rate;
-static unsigned long max_sampler_rate;
-
-static void execute_qsi(void *parms)
-{
-   struct hws_execute_parms *ep = parms;
-
-   ep->rc = 

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Robert Richter]

On 14.06.16 07:36:08, Heiko Carstens wrote:
> On Mon, Jun 13, 2016 at 06:29:14PM +0200, Robert Richter wrote:
> > Heiko,
> > 
> > On 09.06.16 11:00:56, Heiko Carstens wrote:
> > > However I'm wondering if we shouldn't simply remove at least the s390
> > > specific hwswampler code from the oprofile module. This would still leave
> > > the common code timer based sampling mode for oprofile working on s390.
> > > 
> > > It looks like the oprofile user space utility nowadays (since 2012) uses
> > > the kernel perf interface instead of the oprofile interface anyway, if
> > > present. So the oprofile module itself doesn't seem to have too many users
> > > left.
> > > 
> > > Any opinions?
> > 
> > yes, the kernel driver is not necessary for oprofile userland for a
> > while now. There is no ongoing development any longer, most patches
> > are due to changes in the kernel apis.
> > 
> > So if there is code that needs a larger rework due to other kernel
> > changes and there is no user anymore, I am fine with removing the code
> > instead of reworking it. I still would just keep existing code as long
> > as we can keep it unchanged (some like the lightwight of oprofile,
> > esp. in the embedded space). If there is a user of the code, a
> > Tested-by would be good for new code changes.
> > 
> > If there are users of the hwswampler, speak up now. Else, let's just
> > remove it.
> 
> Ok, so I'll wait a week or so and remove the code if nobody speaks up. Is
> it ok for you if I add the patch to the s390 kernel tree?

Yes, pass it through your tree.

> The patch would only remove s390 specific architecture code.
> 
> I have this pending:
> 
> s390/oprofile: remove hardware sampler support
> 
> Remove hardware sampler support from oprofile module.
> 
> The oprofile user space utilty has been switched to use the kernel
> perf interface, for which we also provide hardware sampling support.
> 
> In addition the hardware sampling support is also slightly broken: it
> supports only 16 bits for the pid and therefore would generate wrong
> results on machines which have a pid >64k.
> 
> Also the pt_regs structure which was passed to oprofile common code
> cannot necessarily be used to generate sane backtraces, since the
> task(s) in question may run while the samples are fed to oprofile.
> So the result would be more or less random.
> 
> However given that the only user space tools switched to the perf
> interface already four years ago the hardware sampler code seems to be
> unused code, and therefore it should be reasonable to remove it.
> 
> The timer based oprofile support continues to work.
> 
> Signed-off-by: Heiko Carstens 

Acked-by: Robert Richter 

> 
>  Documentation/kernel-parameters.txt |2 -
>  arch/s390/oprofile/Makefile |1 -
>  arch/s390/oprofile/hwsampler.c  | 1178 
>  arch/s390/oprofile/hwsampler.h  |   63 --
>  arch/s390/oprofile/init.c   |  489 -
>  arch/s390/oprofile/op_counter.h |   21 -
>  6 files changed, 1754 deletions(-)

Could you send the patch for review anyway?

Thanks,

-Robert

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Robert Richter]

On 14.06.16 07:36:08, Heiko Carstens wrote:
> On Mon, Jun 13, 2016 at 06:29:14PM +0200, Robert Richter wrote:
> > Heiko,
> > 
> > On 09.06.16 11:00:56, Heiko Carstens wrote:
> > > However I'm wondering if we shouldn't simply remove at least the s390
> > > specific hwswampler code from the oprofile module. This would still leave
> > > the common code timer based sampling mode for oprofile working on s390.
> > > 
> > > It looks like the oprofile user space utility nowadays (since 2012) uses
> > > the kernel perf interface instead of the oprofile interface anyway, if
> > > present. So the oprofile module itself doesn't seem to have too many users
> > > left.
> > > 
> > > Any opinions?
> > 
> > yes, the kernel driver is not necessary for oprofile userland for a
> > while now. There is no ongoing development any longer, most patches
> > are due to changes in the kernel apis.
> > 
> > So if there is code that needs a larger rework due to other kernel
> > changes and there is no user anymore, I am fine with removing the code
> > instead of reworking it. I still would just keep existing code as long
> > as we can keep it unchanged (some like the lightwight of oprofile,
> > esp. in the embedded space). If there is a user of the code, a
> > Tested-by would be good for new code changes.
> > 
> > If there are users of the hwswampler, speak up now. Else, let's just
> > remove it.
> 
> Ok, so I'll wait a week or so and remove the code if nobody speaks up. Is
> it ok for you if I add the patch to the s390 kernel tree?

Yes, pass it through your tree.

> The patch would only remove s390 specific architecture code.
> 
> I have this pending:
> 
> s390/oprofile: remove hardware sampler support
> 
> Remove hardware sampler support from oprofile module.
> 
> The oprofile user space utilty has been switched to use the kernel
> perf interface, for which we also provide hardware sampling support.
> 
> In addition the hardware sampling support is also slightly broken: it
> supports only 16 bits for the pid and therefore would generate wrong
> results on machines which have a pid >64k.
> 
> Also the pt_regs structure which was passed to oprofile common code
> cannot necessarily be used to generate sane backtraces, since the
> task(s) in question may run while the samples are fed to oprofile.
> So the result would be more or less random.
> 
> However given that the only user space tools switched to the perf
> interface already four years ago the hardware sampler code seems to be
> unused code, and therefore it should be reasonable to remove it.
> 
> The timer based oprofile support continues to work.
> 
> Signed-off-by: Heiko Carstens 

Acked-by: Robert Richter 

> 
>  Documentation/kernel-parameters.txt |2 -
>  arch/s390/oprofile/Makefile |1 -
>  arch/s390/oprofile/hwsampler.c  | 1178 
>  arch/s390/oprofile/hwsampler.h  |   63 --
>  arch/s390/oprofile/init.c   |  489 -
>  arch/s390/oprofile/op_counter.h |   21 -
>  6 files changed, 1754 deletions(-)

Could you send the patch for review anyway?

Thanks,

-Robert

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Heiko Carstens]

On Mon, Jun 13, 2016 at 06:29:14PM +0200, Robert Richter wrote:
> Heiko,
> 
> On 09.06.16 11:00:56, Heiko Carstens wrote:
> > However I'm wondering if we shouldn't simply remove at least the s390
> > specific hwswampler code from the oprofile module. This would still leave
> > the common code timer based sampling mode for oprofile working on s390.
> > 
> > It looks like the oprofile user space utility nowadays (since 2012) uses
> > the kernel perf interface instead of the oprofile interface anyway, if
> > present. So the oprofile module itself doesn't seem to have too many users
> > left.
> > 
> > Any opinions?
> 
> yes, the kernel driver is not necessary for oprofile userland for a
> while now. There is no ongoing development any longer, most patches
> are due to changes in the kernel apis.
> 
> So if there is code that needs a larger rework due to other kernel
> changes and there is no user anymore, I am fine with removing the code
> instead of reworking it. I still would just keep existing code as long
> as we can keep it unchanged (some like the lightwight of oprofile,
> esp. in the embedded space). If there is a user of the code, a
> Tested-by would be good for new code changes.
> 
> If there are users of the hwswampler, speak up now. Else, let's just
> remove it.

Ok, so I'll wait a week or so and remove the code if nobody speaks up. Is
it ok for you if I add the patch to the s390 kernel tree?
The patch would only remove s390 specific architecture code.

I have this pending:

s390/oprofile: remove hardware sampler support

Remove hardware sampler support from oprofile module.

The oprofile user space utilty has been switched to use the kernel
perf interface, for which we also provide hardware sampling support.

In addition the hardware sampling support is also slightly broken: it
supports only 16 bits for the pid and therefore would generate wrong
results on machines which have a pid >64k.

Also the pt_regs structure which was passed to oprofile common code
cannot necessarily be used to generate sane backtraces, since the
task(s) in question may run while the samples are fed to oprofile.
So the result would be more or less random.

However given that the only user space tools switched to the perf
interface already four years ago the hardware sampler code seems to be
unused code, and therefore it should be reasonable to remove it.

The timer based oprofile support continues to work.

Signed-off-by: Heiko Carstens 

 Documentation/kernel-parameters.txt |2 -
 arch/s390/oprofile/Makefile |1 -
 arch/s390/oprofile/hwsampler.c  | 1178 
 arch/s390/oprofile/hwsampler.h  |   63 --
 arch/s390/oprofile/init.c   |  489 -
 arch/s390/oprofile/op_counter.h |   21 -
 6 files changed, 1754 deletions(-)

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Heiko Carstens]

On Mon, Jun 13, 2016 at 06:29:14PM +0200, Robert Richter wrote:
> Heiko,
> 
> On 09.06.16 11:00:56, Heiko Carstens wrote:
> > However I'm wondering if we shouldn't simply remove at least the s390
> > specific hwswampler code from the oprofile module. This would still leave
> > the common code timer based sampling mode for oprofile working on s390.
> > 
> > It looks like the oprofile user space utility nowadays (since 2012) uses
> > the kernel perf interface instead of the oprofile interface anyway, if
> > present. So the oprofile module itself doesn't seem to have too many users
> > left.
> > 
> > Any opinions?
> 
> yes, the kernel driver is not necessary for oprofile userland for a
> while now. There is no ongoing development any longer, most patches
> are due to changes in the kernel apis.
> 
> So if there is code that needs a larger rework due to other kernel
> changes and there is no user anymore, I am fine with removing the code
> instead of reworking it. I still would just keep existing code as long
> as we can keep it unchanged (some like the lightwight of oprofile,
> esp. in the embedded space). If there is a user of the code, a
> Tested-by would be good for new code changes.
> 
> If there are users of the hwswampler, speak up now. Else, let's just
> remove it.

Ok, so I'll wait a week or so and remove the code if nobody speaks up. Is
it ok for you if I add the patch to the s390 kernel tree?
The patch would only remove s390 specific architecture code.

I have this pending:

s390/oprofile: remove hardware sampler support

Remove hardware sampler support from oprofile module.

The oprofile user space utilty has been switched to use the kernel
perf interface, for which we also provide hardware sampling support.

In addition the hardware sampling support is also slightly broken: it
supports only 16 bits for the pid and therefore would generate wrong
results on machines which have a pid >64k.

Also the pt_regs structure which was passed to oprofile common code
cannot necessarily be used to generate sane backtraces, since the
task(s) in question may run while the samples are fed to oprofile.
So the result would be more or less random.

However given that the only user space tools switched to the perf
interface already four years ago the hardware sampler code seems to be
unused code, and therefore it should be reasonable to remove it.

The timer based oprofile support continues to work.

Signed-off-by: Heiko Carstens 

 Documentation/kernel-parameters.txt |2 -
 arch/s390/oprofile/Makefile |1 -
 arch/s390/oprofile/hwsampler.c  | 1178 
 arch/s390/oprofile/hwsampler.h  |   63 --
 arch/s390/oprofile/init.c   |  489 -
 arch/s390/oprofile/op_counter.h |   21 -
 6 files changed, 1754 deletions(-)

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[William Cohen]

On 06/13/2016 12:29 PM, Robert Richter wrote:
> Heiko,
> 
> On 09.06.16 11:00:56, Heiko Carstens wrote:
>> However I'm wondering if we shouldn't simply remove at least the s390
>> specific hwswampler code from the oprofile module. This would still leave
>> the common code timer based sampling mode for oprofile working on s390.
>>
>> It looks like the oprofile user space utility nowadays (since 2012) uses
>> the kernel perf interface instead of the oprofile interface anyway, if
>> present. So the oprofile module itself doesn't seem to have too many users
>> left.
>>
>> Any opinions?
> 
> yes, the kernel driver is not necessary for oprofile userland for a
> while now. There is no ongoing development any longer, most patches
> are due to changes in the kernel apis.
> 
> So if there is code that needs a larger rework due to other kernel
> changes and there is no user anymore, I am fine with removing the code
> instead of reworking it. I still would just keep existing code as long
> as we can keep it unchanged (some like the lightwight of oprofile,
> esp. in the embedded space). If there is a user of the code, a
> Tested-by would be good for new code changes.
> 
> If there are users of the hwswampler, speak up now. Else, let's just
> remove it.
> 
> -Robert
> 

Hi,

As Robert mentioned the user-space oprofile code that would have used the 
oprofile device driver was removed around August 2014 in preparation for 
oprofile-1.0. The operf command which uses the kernel perf infrastructure and 
does not need for the oprofile kernel driver has been in oprofile since August 
of 2012.  For some architectures it would make sense to simplify things by 
eliminate the oprofile kernel driver.

-Will Cohen

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[William Cohen]

On 06/13/2016 12:29 PM, Robert Richter wrote:
> Heiko,
> 
> On 09.06.16 11:00:56, Heiko Carstens wrote:
>> However I'm wondering if we shouldn't simply remove at least the s390
>> specific hwswampler code from the oprofile module. This would still leave
>> the common code timer based sampling mode for oprofile working on s390.
>>
>> It looks like the oprofile user space utility nowadays (since 2012) uses
>> the kernel perf interface instead of the oprofile interface anyway, if
>> present. So the oprofile module itself doesn't seem to have too many users
>> left.
>>
>> Any opinions?
> 
> yes, the kernel driver is not necessary for oprofile userland for a
> while now. There is no ongoing development any longer, most patches
> are due to changes in the kernel apis.
> 
> So if there is code that needs a larger rework due to other kernel
> changes and there is no user anymore, I am fine with removing the code
> instead of reworking it. I still would just keep existing code as long
> as we can keep it unchanged (some like the lightwight of oprofile,
> esp. in the embedded space). If there is a user of the code, a
> Tested-by would be good for new code changes.
> 
> If there are users of the hwswampler, speak up now. Else, let's just
> remove it.
> 
> -Robert
> 

Hi,

As Robert mentioned the user-space oprofile code that would have used the 
oprofile device driver was removed around August 2014 in preparation for 
oprofile-1.0. The operf command which uses the kernel perf infrastructure and 
does not need for the oprofile kernel driver has been in oprofile since August 
of 2012.  For some architectures it would make sense to simplify things by 
eliminate the oprofile kernel driver.

-Will Cohen

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Robert Richter]

Heiko,

On 09.06.16 11:00:56, Heiko Carstens wrote:
> However I'm wondering if we shouldn't simply remove at least the s390
> specific hwswampler code from the oprofile module. This would still leave
> the common code timer based sampling mode for oprofile working on s390.
> 
> It looks like the oprofile user space utility nowadays (since 2012) uses
> the kernel perf interface instead of the oprofile interface anyway, if
> present. So the oprofile module itself doesn't seem to have too many users
> left.
> 
> Any opinions?

yes, the kernel driver is not necessary for oprofile userland for a
while now. There is no ongoing development any longer, most patches
are due to changes in the kernel apis.

So if there is code that needs a larger rework due to other kernel
changes and there is no user anymore, I am fine with removing the code
instead of reworking it. I still would just keep existing code as long
as we can keep it unchanged (some like the lightwight of oprofile,
esp. in the embedded space). If there is a user of the code, a
Tested-by would be good for new code changes.

If there are users of the hwswampler, speak up now. Else, let's just
remove it.

-Robert

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Robert Richter]

Heiko,

On 09.06.16 11:00:56, Heiko Carstens wrote:
> However I'm wondering if we shouldn't simply remove at least the s390
> specific hwswampler code from the oprofile module. This would still leave
> the common code timer based sampling mode for oprofile working on s390.
> 
> It looks like the oprofile user space utility nowadays (since 2012) uses
> the kernel perf interface instead of the oprofile interface anyway, if
> present. So the oprofile module itself doesn't seem to have too many users
> left.
> 
> Any opinions?

yes, the kernel driver is not necessary for oprofile userland for a
while now. There is no ongoing development any longer, most patches
are due to changes in the kernel apis.

So if there is code that needs a larger rework due to other kernel
changes and there is no user anymore, I am fine with removing the code
instead of reworking it. I still would just keep existing code as long
as we can keep it unchanged (some like the lightwight of oprofile,
esp. in the embedded space). If there is a user of the code, a
Tested-by would be good for new code changes.

If there are users of the hwswampler, speak up now. Else, let's just
remove it.

-Robert

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Heiko Carstens]

On Wed, Jun 08, 2016 at 03:29:12AM +0530, Bhaktipriya Shridhar wrote:
> A dedicated workqueue has been used since the workqueue hws_wq with
> workitem >worker, is involved in hardware based sampling
> on System z processors.
> 
> Since, these are long-running work items and aren't involved in memory
> reclaim in any way, system_long_wq has been used.
> 
> Signed-off-by: Bhaktipriya Shridhar 
> ---
>  arch/s390/oprofile/hwsampler.c | 21 +++--
>  1 file changed, 3 insertions(+), 18 deletions(-)

Besides that your patch doesn't compile... ;)

Removing the flush_workqueue is probably wrong, since the oprofile module
can now be unloaded while there could still be a reference to a worker
structure owned by the oprofile module.

However I'm wondering if we shouldn't simply remove at least the s390
specific hwswampler code from the oprofile module. This would still leave
the common code timer based sampling mode for oprofile working on s390.

It looks like the oprofile user space utility nowadays (since 2012) uses
the kernel perf interface instead of the oprofile interface anyway, if
present. So the oprofile module itself doesn't seem to have too many users
left.

Any opinions?

> 
> - if (hws_wq)
> - flush_workqueue(hws_wq);
> -
>   mutex_lock(_sem);
> 
>   if (hws_state == HWS_STOPPED) {
> @@ -1059,10 +1048,6 @@ int hwsampler_shutdown(void)
>   hws_alert = 0;
>   deallocate_sdbt();
>   }
> - if (hws_wq) {
> - destroy_workqueue(hws_wq);
> - hws_wq = NULL;
> - }
> 
>   unregister_external_irq(EXT_IRQ_MEASURE_ALERT, hws_ext_handler);
>   hws_state = HWS_INIT;
> --
> 2.1.4
> 

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[Heiko Carstens]

On Wed, Jun 08, 2016 at 03:29:12AM +0530, Bhaktipriya Shridhar wrote:
> A dedicated workqueue has been used since the workqueue hws_wq with
> workitem >worker, is involved in hardware based sampling
> on System z processors.
> 
> Since, these are long-running work items and aren't involved in memory
> reclaim in any way, system_long_wq has been used.
> 
> Signed-off-by: Bhaktipriya Shridhar 
> ---
>  arch/s390/oprofile/hwsampler.c | 21 +++--
>  1 file changed, 3 insertions(+), 18 deletions(-)

Besides that your patch doesn't compile... ;)

Removing the flush_workqueue is probably wrong, since the oprofile module
can now be unloaded while there could still be a reference to a worker
structure owned by the oprofile module.

However I'm wondering if we shouldn't simply remove at least the s390
specific hwswampler code from the oprofile module. This would still leave
the common code timer based sampling mode for oprofile working on s390.

It looks like the oprofile user space utility nowadays (since 2012) uses
the kernel perf interface instead of the oprofile interface anyway, if
present. So the oprofile module itself doesn't seem to have too many users
left.

Any opinions?

> 
> - if (hws_wq)
> - flush_workqueue(hws_wq);
> -
>   mutex_lock(_sem);
> 
>   if (hws_state == HWS_STOPPED) {
> @@ -1059,10 +1048,6 @@ int hwsampler_shutdown(void)
>   hws_alert = 0;
>   deallocate_sdbt();
>   }
> - if (hws_wq) {
> - destroy_workqueue(hws_wq);
> - hws_wq = NULL;
> - }
> 
>   unregister_external_irq(EXT_IRQ_MEASURE_ALERT, hws_ext_handler);
>   hws_state = HWS_INIT;
> --
> 2.1.4
> 

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[kbuild test robot]

Hi,

[auto build test ERROR on s390/features]
[also build test ERROR on v4.7-rc2 next-20160607]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Bhaktipriya-Shridhar/s390-oprofile-Remove-deprecated-create_workqueue/20160608-060615
base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
config: s390-default_defconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390 

All errors (new ones prefixed by >>):

   arch/s390/oprofile/hwsampler.c: In function 'hwsampler_stop_all':
>> arch/s390/oprofile/hwsampler.c:1151:6: error: 'hws_wq' undeclared (first use 
>> in this function)
 if (hws_wq)
 ^
   arch/s390/oprofile/hwsampler.c:1151:6: note: each undeclared identifier is 
reported only once for each function it appears in

vim +/hws_wq +1151 arch/s390/oprofile/hwsampler.c

ec6a3df1 Heinz Graalfs 2011-01-21  1145 cb->stop_mode = 1;
ec6a3df1 Heinz Graalfs 2011-01-21  1146 tmp_rc = 
stop_sampling(cpu);
ec6a3df1 Heinz Graalfs 2011-01-21  1147 if (tmp_rc)
ec6a3df1 Heinz Graalfs 2011-01-21  1148 rc = tmp_rc;
ec6a3df1 Heinz Graalfs 2011-01-21  1149 }
ec6a3df1 Heinz Graalfs 2011-01-21  1150  
ec6a3df1 Heinz Graalfs 2011-01-21 @1151 if (hws_wq)
ec6a3df1 Heinz Graalfs 2011-01-21  1152 flush_workqueue(hws_wq);
ec6a3df1 Heinz Graalfs 2011-01-21  1153  
ec6a3df1 Heinz Graalfs 2011-01-21  1154 mutex_lock(_sem);

:: The code at line 1151 was first introduced by commit
:: ec6a3df1c008d9e8664e53b0363f6847c5c0dc3f oprofile, s390: Add support for 
hardware based sampling on System z processors

:: TO: Heinz Graalfs 
:: CC: Robert Richter 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue

[kbuild test robot]

Hi,

[auto build test ERROR on s390/features]
[also build test ERROR on v4.7-rc2 next-20160607]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Bhaktipriya-Shridhar/s390-oprofile-Remove-deprecated-create_workqueue/20160608-060615
base:   https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
config: s390-default_defconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390 

All errors (new ones prefixed by >>):

   arch/s390/oprofile/hwsampler.c: In function 'hwsampler_stop_all':
>> arch/s390/oprofile/hwsampler.c:1151:6: error: 'hws_wq' undeclared (first use 
>> in this function)
 if (hws_wq)
 ^
   arch/s390/oprofile/hwsampler.c:1151:6: note: each undeclared identifier is 
reported only once for each function it appears in

vim +/hws_wq +1151 arch/s390/oprofile/hwsampler.c

ec6a3df1 Heinz Graalfs 2011-01-21  1145 cb->stop_mode = 1;
ec6a3df1 Heinz Graalfs 2011-01-21  1146 tmp_rc = 
stop_sampling(cpu);
ec6a3df1 Heinz Graalfs 2011-01-21  1147 if (tmp_rc)
ec6a3df1 Heinz Graalfs 2011-01-21  1148 rc = tmp_rc;
ec6a3df1 Heinz Graalfs 2011-01-21  1149 }
ec6a3df1 Heinz Graalfs 2011-01-21  1150  
ec6a3df1 Heinz Graalfs 2011-01-21 @1151 if (hws_wq)
ec6a3df1 Heinz Graalfs 2011-01-21  1152 flush_workqueue(hws_wq);
ec6a3df1 Heinz Graalfs 2011-01-21  1153  
ec6a3df1 Heinz Graalfs 2011-01-21  1154 mutex_lock(_sem);

:: The code at line 1151 was first introduced by commit
:: ec6a3df1c008d9e8664e53b0363f6847c5c0dc3f oprofile, s390: Add support for 
hardware based sampling on System z processors

:: TO: Heinz Graalfs 
:: CC: Robert Richter 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

[PATCH] s390/oprofile: Remove deprecated create_workqueue

[Bhaktipriya Shridhar]

A dedicated workqueue has been used since the workqueue hws_wq with
workitem >worker, is involved in hardware based sampling
on System z processors.

Since, these are long-running work items and aren't involved in memory
reclaim in any way, system_long_wq has been used.

Signed-off-by: Bhaktipriya Shridhar 
---
 arch/s390/oprofile/hwsampler.c | 21 +++--
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/arch/s390/oprofile/hwsampler.c b/arch/s390/oprofile/hwsampler.c
index ff9b4eb..8b16bb8 100644
--- a/arch/s390/oprofile/hwsampler.c
+++ b/arch/s390/oprofile/hwsampler.c
@@ -42,7 +42,6 @@ static DEFINE_MUTEX(hws_sem_oom);
 static unsigned char hws_flush_all;
 static unsigned int hws_oom;
 static unsigned int hws_alert;
-static struct workqueue_struct *hws_wq;

 static unsigned int hws_state;
 enum {
@@ -189,8 +188,7 @@ static void hws_ext_handler(struct ext_code ext_code,
inc_irq_stat(IRQEXT_CMS);
atomic_xchg(>ext_params, atomic_read(>ext_params) | param32);

-   if (hws_wq)
-   queue_work(hws_wq, >worker);
+   queue_work(system_long_wq, >worker);
 }

 static void worker(struct work_struct *work);
@@ -566,15 +564,12 @@ int hwsampler_deactivate(unsigned int cpu)
} else  {
hws_flush_all = 1;
/* Add work to queue to read pending samples.*/
-   queue_work_on(cpu, hws_wq, >worker);
+   queue_work_on(cpu, system_long_wq, >worker);
}
}
}
mutex_unlock(_sem);

-   if (hws_wq)
-   flush_workqueue(hws_wq);
-
return rc;
 }

@@ -740,7 +735,7 @@ static void worker_on_finish(unsigned int cpu)
continue;
if (!cb->finish) {
cb->finish = 1;
-   queue_work_on(i, hws_wq,
+   queue_work_on(i, system_long_wq,
>worker);
}
}
@@ -997,9 +992,6 @@ int hwsampler_setup(void)
goto setup_exit;

rc = -EINVAL;
-   hws_wq = create_workqueue("hwsampler");
-   if (!hws_wq)
-   goto setup_exit;

register_cpu_notifier(_cpu_notifier);

@@ -1049,9 +1041,6 @@ int hwsampler_shutdown(void)
if (hws_state == HWS_DEALLOCATED || hws_state == HWS_STOPPED) {
mutex_unlock(_sem);

-   if (hws_wq)
-   flush_workqueue(hws_wq);
-
mutex_lock(_sem);

if (hws_state == HWS_STOPPED) {
@@ -1059,10 +1048,6 @@ int hwsampler_shutdown(void)
hws_alert = 0;
deallocate_sdbt();
}
-   if (hws_wq) {
-   destroy_workqueue(hws_wq);
-   hws_wq = NULL;
-   }

unregister_external_irq(EXT_IRQ_MEASURE_ALERT, hws_ext_handler);
hws_state = HWS_INIT;
--
2.1.4

[PATCH] s390/oprofile: Remove deprecated create_workqueue

[Bhaktipriya Shridhar]

A dedicated workqueue has been used since the workqueue hws_wq with
workitem >worker, is involved in hardware based sampling
on System z processors.

Since, these are long-running work items and aren't involved in memory
reclaim in any way, system_long_wq has been used.

Signed-off-by: Bhaktipriya Shridhar 
---
 arch/s390/oprofile/hwsampler.c | 21 +++--
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/arch/s390/oprofile/hwsampler.c b/arch/s390/oprofile/hwsampler.c
index ff9b4eb..8b16bb8 100644
--- a/arch/s390/oprofile/hwsampler.c
+++ b/arch/s390/oprofile/hwsampler.c
@@ -42,7 +42,6 @@ static DEFINE_MUTEX(hws_sem_oom);
 static unsigned char hws_flush_all;
 static unsigned int hws_oom;
 static unsigned int hws_alert;
-static struct workqueue_struct *hws_wq;

 static unsigned int hws_state;
 enum {
@@ -189,8 +188,7 @@ static void hws_ext_handler(struct ext_code ext_code,
inc_irq_stat(IRQEXT_CMS);
atomic_xchg(>ext_params, atomic_read(>ext_params) | param32);

-   if (hws_wq)
-   queue_work(hws_wq, >worker);
+   queue_work(system_long_wq, >worker);
 }

 static void worker(struct work_struct *work);
@@ -566,15 +564,12 @@ int hwsampler_deactivate(unsigned int cpu)
} else  {
hws_flush_all = 1;
/* Add work to queue to read pending samples.*/
-   queue_work_on(cpu, hws_wq, >worker);
+   queue_work_on(cpu, system_long_wq, >worker);
}
}
}
mutex_unlock(_sem);

-   if (hws_wq)
-   flush_workqueue(hws_wq);
-
return rc;
 }

@@ -740,7 +735,7 @@ static void worker_on_finish(unsigned int cpu)
continue;
if (!cb->finish) {
cb->finish = 1;
-   queue_work_on(i, hws_wq,
+   queue_work_on(i, system_long_wq,
>worker);
}
}
@@ -997,9 +992,6 @@ int hwsampler_setup(void)
goto setup_exit;

rc = -EINVAL;
-   hws_wq = create_workqueue("hwsampler");
-   if (!hws_wq)
-   goto setup_exit;

register_cpu_notifier(_cpu_notifier);

@@ -1049,9 +1041,6 @@ int hwsampler_shutdown(void)
if (hws_state == HWS_DEALLOCATED || hws_state == HWS_STOPPED) {
mutex_unlock(_sem);

-   if (hws_wq)
-   flush_workqueue(hws_wq);
-
mutex_lock(_sem);

if (hws_state == HWS_STOPPED) {
@@ -1059,10 +1048,6 @@ int hwsampler_shutdown(void)
hws_alert = 0;
deallocate_sdbt();
}
-   if (hws_wq) {
-   destroy_workqueue(hws_wq);
-   hws_wq = NULL;
-   }

unregister_external_irq(EXT_IRQ_MEASURE_ALERT, hws_ext_handler);
hws_state = HWS_INIT;
--
2.1.4