Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1416656 > unrolled thread

[PATCH] s390/oprofile: Remove deprecated create_workqueue

Back to article view | Back to linux.kernel

Contents

  [PATCH] s390/oprofile: Remove deprecated create_workqueue Bhaktipriya Shridhar <bhaktipriya96@gmail.com> - 2016-06-08 00:00 +0200
    Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue kbuild test robot <lkp@intel.com> - 2016-06-08 01:30 +0200
    Re: [PATCH] s390/oprofile: Remove deprecated create_workqueue Heiko Carstens <heiko.carstens@de.ibm.com> - 2016-06-09 11:10 +0200

#1416656 — [PATCH] 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.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
 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(&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

[toc] | [next] | [standalone]

#1416726

[Multipart message — attachments visible in raw view] — view raw

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 <graalfs@linux.vnet.ibm.com>
:::::: CC: Robert Richter <robert.richter@amd.com>

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

[toc] | [prev] | [next] | [standalone]

#1418184

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 &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.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
>  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(&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
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web