Re: [PATCH] padata: Use RCU when fetching pd from do_serial

From: Peter Zijlstra
Date: Tue Jul 16 2019 - 09:15:40 EST


On Tue, Jul 16, 2019 at 08:57:04PM +0800, Herbert Xu wrote:
> On Tue, Jul 16, 2019 at 01:14:10PM +0200, Steffen Klassert wrote:
> >
> > Maybe we can fix it if we call padata_free_pd() from
> > padata_serial_worker() when it sent out the last object.
>
> How about using RCU?
>
> We still need to fix up the refcnt if it's supposed to limit the
> overall number of outstanding requests.
>
> ---8<---
> The function padata_do_serial uses parallel_data without obeying
> the RCU rules around its life-cycle. This means that a concurrent
> padata_replace call can result in a crash.
>
> This patch fixes it by using RCU just as we do in padata_do_parallel.
>
> Fixes: 16295bec6398 ("padata: Generic parallelization/...")
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

> diff --git a/kernel/padata.c b/kernel/padata.c
> index 2d2fddbb7a4c..fb5dd1210d2b 100644
> --- a/kernel/padata.c
> +++ b/kernel/padata.c
> @@ -128,7 +128,7 @@ int padata_do_parallel(struct padata_instance *pinst,
>
> err = 0;
> atomic_inc(&pd->refcnt);
> - padata->pd = pd;
> + padata->inst = pinst;
> padata->cb_cpu = cb_cpu;
>
> target_cpu = padata_cpu_hash(pd);
> @@ -367,7 +368,7 @@ void padata_do_serial(struct padata_priv *padata)
> struct parallel_data *pd;
> int reorder_via_wq = 0;
>
> - pd = padata->pd;
> + pd = rcu_dereference_bh(padata->inst->pd);
>
> cpu = get_cpu();
>

That's weird for not having a matching assign and lacking comments to
explain that.