linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RT] padata: Make padata_do_serial() use get_cpu_light()
@ 2019-01-09 15:59 Daniel Bristot de Oliveira
  2019-01-14 10:05 ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Bristot de Oliveira @ 2019-01-09 15:59 UTC (permalink / raw)
  To: linux-rt-users
  Cc: Sebastian Andrzej Siewior, Thomas Gleixner, Clark Williams,
	linux-crypto, linux-kernel

We hit the following BUG:

  BUG: scheduling while atomic: kworker/1:1/24117/0x00000002
  Preemption disabled at:
  [<ffffffffb61fd824>] padata_do_serial+0x24/0x110
  CPU: 1 PID: 24117 Comm: kworker/1:1 Not tainted 4.18.0-56.rt9.107.el8.x86_64 #1
  Hardware name: HPE ProLiant DL380 Gen10/ProLiant DL380 Gen10, BIOS U30 11/14/2017
  Workqueue: pencrypt padata_parallel_worker
  Call Trace:
    dump_stack+0x5c/0x80
    ? padata_do_serial+0x24/0x110
    __schedule_bug.cold.83+0x8e/0x9b
    __schedule+0x5a0/0x680
    schedule+0x39/0xd0
    rt_spin_lock_slowlock_locked+0x10e/0x2b0
    rt_spin_lock_slowlock+0x50/0x80
    padata_do_serial+0x4d/0x110
    padata_parallel_worker+0xaf/0xe0
    process_one_work+0x183/0x3b0
    ? process_one_work+0x3b0/0x3b0
    worker_thread+0x30/0x3d0
    ? process_one_work+0x3b0/0x3b0
    kthread+0x112/0x130
    ? kthread_create_worker_on_cpu+0x70/0x70
    ret_from_fork+0x35/0x40

and the cause is a spin_lock() taken inside a get_cpu() section.

Convert the get/put_cpu to get/put_cpu_light to fix the BUG while reducing the
preempt_disable section.

Signed-off-by: Daniel Bristot de Oliveira <bristot@redhat.com>
Reviewed-by: Clark Williams <williams@redhat.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Clark Williams <williams@redhat.com>
Cc: linux-rt-users@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

diff --git a/kernel/padata.c b/kernel/padata.c
index d568cc56405f..bfcbdeb20ba5 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -295,7 +295,7 @@ static void padata_reorder_timer(struct timer_list *t)
 	unsigned int weight;
 	int target_cpu, cpu;
 
-	cpu = get_cpu();
+	cpu = get_cpu_light();
 
 	/* We don't lock pd here to not interfere with parallel processing
 	 * padata_reorder() calls on other CPUs. We just need any CPU out of
@@ -321,7 +321,7 @@ static void padata_reorder_timer(struct timer_list *t)
 		padata_reorder(pd);
 	}
 
-	put_cpu();
+	put_cpu_light();
 }
 
 static void padata_serial_worker(struct work_struct *serial_work)
@@ -369,7 +369,7 @@ void padata_do_serial(struct padata_priv *padata)
 
 	pd = padata->pd;
 
-	cpu = get_cpu();
+	cpu = get_cpu_light();
 
 	/* We need to run on the same CPU padata_do_parallel(.., padata, ..)
 	 * was called on -- or, at least, enqueue the padata object into the
@@ -387,7 +387,7 @@ void padata_do_serial(struct padata_priv *padata)
 	list_add_tail(&padata->list, &pqueue->reorder.list);
 	spin_unlock(&pqueue->reorder.lock);
 
-	put_cpu();
+	put_cpu_light();
 
 	/* If we're running on the wrong CPU, call padata_reorder() via a
 	 * kernel worker.
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH RT] padata: Make padata_do_serial() use get_cpu_light()
  2019-01-09 15:59 [PATCH RT] padata: Make padata_do_serial() use get_cpu_light() Daniel Bristot de Oliveira
@ 2019-01-14 10:05 ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 2+ messages in thread
From: Sebastian Andrzej Siewior @ 2019-01-14 10:05 UTC (permalink / raw)
  To: Daniel Bristot de Oliveira
  Cc: linux-rt-users, Thomas Gleixner, Clark Williams, linux-crypto,
	linux-kernel

On 2019-01-09 16:59:26 [+0100], Daniel Bristot de Oliveira wrote:
> diff --git a/kernel/padata.c b/kernel/padata.c
> index d568cc56405f..bfcbdeb20ba5 100644
> --- a/kernel/padata.c
> +++ b/kernel/padata.c
> @@ -295,7 +295,7 @@ static void padata_reorder_timer(struct timer_list *t)
>  	unsigned int weight;
>  	int target_cpu, cpu;
>  
> -	cpu = get_cpu();
> +	cpu = get_cpu_light();

This can become a
	cpu = smp_processor_id()

>  	/* We don't lock pd here to not interfere with parallel processing
>  	 * padata_reorder() calls on other CPUs. We just need any CPU out of
> @@ -321,7 +321,7 @@ static void padata_reorder_timer(struct timer_list *t)
>  		padata_reorder(pd);
>  	}
>  
> -	put_cpu();
> +	put_cpu_light();

and this can go because this is invoked in a timer callback.

>  }
>  
>  static void padata_serial_worker(struct work_struct *serial_work)
> @@ -369,7 +369,7 @@ void padata_do_serial(struct padata_priv *padata)
>  
>  	pd = padata->pd;
>  
> -	cpu = get_cpu();
> +	cpu = get_cpu_light();

this is tricky but I would also say that this can become
smp_processor_id() like in the upper hunk

>  	/* We need to run on the same CPU padata_do_parallel(.., padata, ..)
>  	 * was called on -- or, at least, enqueue the padata object into the
> @@ -387,7 +387,7 @@ void padata_do_serial(struct padata_priv *padata)
>  	list_add_tail(&padata->list, &pqueue->reorder.list);
>  	spin_unlock(&pqueue->reorder.lock);
>  
> -	put_cpu();
> +	put_cpu_light();

and than this can go, too. It looks like this invoked from a worker with
BH disabled. If it does, it is all good. If not then it might be
problematic because later we have
	queue_work_on(cpu, pd->pinst->wq, &pqueue->reorder_work);

and nothing guarantees that the work is carried out by the CPU specified
if CPU-hotplug is involved.

>  	/* If we're running on the wrong CPU, call padata_reorder() via a
>  	 * kernel worker.

Sebastian

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-01-14 10:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 15:59 [PATCH RT] padata: Make padata_do_serial() use get_cpu_light() Daniel Bristot de Oliveira
2019-01-14 10:05 ` Sebastian Andrzej Siewior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).