s390/oprofile: Remove deprecated create_workqueue
Subsystems (2)
PATCH 1/1 s390/oprofile: Remove deprecated create_workqueue
A dedicated workqueue has been used since the workqueue hws_wq with workitem &cb->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.
--- arch/s390/oprofile/hwsampler.c | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-)
arch/s390/oprofile/hwsampler.c +3 -18
@@ -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 (&cb->ext_params,atomic_read (&cb->ext_params) | param32); -if (hws_wq) -queue_work (hws_wq, &cb->worker); +queue_work (system_long_wq, &cb->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, &cb->worker); +queue_work_on (cpu, system_long_wq, &cb->worker); } } }mutex_unlock (&hws_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, &cb->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 (&hws_cpu_notifier);
@@ -1049,9 +1041,6 @@ int hwsampler_shutdown(void)if (hws_state ==HWS_DEALLOCATED || hws_state ==HWS_STOPPED ) {mutex_unlock (&hws_sem); -if (hws_wq) -flush_workqueue (hws_wq); -mutex_lock (&hws_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
kbuild test robot
build botreproduce: 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…
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(&hws_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 <[email protected]> :::::: CC: Robert Richter <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Quoted 11 lines · Bhaktipriya Shridhar
A dedicated workqueue has been used since the workqueue
hws_wqwith workitem&cb->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_wqhas been used.Signed-off-by Bhaktipriya Shridhar <[email protected]>--- 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?
Quoted 21 lines
arch/s390/oprofile/hwsampler.c +0
-8-if (hws_wq) -flush_workqueue (hws_wq); -mutex_lock (&hws_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
Heiko,
Quoted 10 lines · Heiko Carstens
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
Quoted 25 lines · Robert Richter
Heiko,
Quoted 10 lines · Heiko Carstens
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.
Quoted 5 lines
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
Quoted 25 lines · Robert Richter
Heiko,
Quoted 10 lines · Heiko Carstens
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.
Quoted 2 lines
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.
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(-)
Robert Richter
Acked-byQuoted 26 lines · Heiko Carstens
Quoted 25 lines · Robert Richter
Heiko,
Quoted 10 lines · Heiko Carstens
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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 20 lines
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_regsstructure 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.
Quoted 7 lines
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 <[email protected]>
Quoted 8 lines
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
Quoted 10 lines · Robert Richter
Quoted 8 lines
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 <[email protected]>
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.
--- 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
Documentation/kernel-parameters.txt +0 -2
@@ -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
arch/s390/oprofile/Makefile +0 -1
@@ -7,4 +7,3 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \ timer_int.o ) oprofile-y := $(DRIVER_OBJS) init.o -oprofile-y += hwsampler.o
arch/s390/oprofile/hwsampler.c deleted +0 -1178
1,178 lines not on this page · raw message
arch/s390/oprofile/hwsampler.h deleted +0 -63
63 lines not on this page · raw message
arch/s390/oprofile/init.c +0 -489
506 lines not on this page · raw message
arch/s390/oprofile/op_counter.h deleted +0 -21
@@ -1,21 +0,0 @@ -/* -* Copyright IBM Corp. 2011 -* Author(s): Andreas Krebbel ([email protected]) -* -* @remark Copyright 2011 OProfile authors -*/ - -#ifndef OP_COUNTER_H -#define OP_COUNTER_H - -struct op_counter_config { -/* `enabled' maps to the hwsampler_file variable. */ -/* `count' maps to the oprofile_hw_interval variable. */ -/* `event' and `unit_mask' are unused. */ -unsigned long kernel; -unsigned long user; -}; - -extern struct op_counter_config counter_config; - -#endif /* OP_COUNTER_H */
Signature · 1 line
2.6.6
Hendrik Brueckner
Reviewed-byQuoted 41 lines · Heiko Carstens
Quoted 10 lines · Robert Richter
Quoted 8 lines
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
74ab0aefe07573358cc6a0c462ddc62ccba4c0cfMon Sep 17 00:00:00 2001
From: Heiko Carstens <[email protected]>
Date: Fri, 10 Jun 2016 10:40:06 +0200
Subject: [PATCH] s390/oprofile: remove hardware sampler supportRemove 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_regsstructure 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 <[email protected]>
Quoted 12 lines
--- 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