linux.git/kernel/padata.c, branch v3.16.81

padata: use smp_mb in padata_reorder to avoid orphaned padata jobs

2019-10-31T22:15:05+00:00

commit cf144f81a99d1a3928f90b0936accfd3f45c9a0a upstream.

Testing padata with the tcrypt module on a 5.2 kernel...

    # modprobe tcrypt alg="pcrypt(rfc4106(gcm(aes)))" type=3
    # modprobe tcrypt mode=211 sec=1

...produces this splat:

    INFO: task modprobe:10075 blocked for more than 120 seconds.
          Not tainted 5.2.0-base+ #16
    modprobe        D    0 10075  10064 0x80004080
    Call Trace:
     ? __schedule+0x4dd/0x610
     ? ring_buffer_unlock_commit+0x23/0x100
     schedule+0x6c/0x90
     schedule_timeout+0x3b/0x320
     ? trace_buffer_unlock_commit_regs+0x4f/0x1f0
     wait_for_common+0x160/0x1a0
     ? wake_up_q+0x80/0x80
     { crypto_wait_req }             # entries in braces added by hand
     { do_one_aead_op }
     { test_aead_jiffies }
     test_aead_speed.constprop.17+0x681/0xf30 [tcrypt]
     do_test+0x4053/0x6a2b [tcrypt]
     ? 0xffffffffa00f4000
     tcrypt_mod_init+0x50/0x1000 [tcrypt]
     ...

The second modprobe command never finishes because in padata_reorder,
CPU0's load of reorder_objects is executed before the unlocking store in
spin_unlock_bh(pd->lock), causing CPU0 to miss CPU1's increment:

CPU0                                 CPU1

padata_reorder                       padata_do_serial
  LOAD reorder_objects  // 0
                                       INC reorder_objects  // 1
                                       padata_reorder
                                         TRYLOCK pd->lock   // failed
  UNLOCK pd->lock

CPU0 deletes the timer before returning from padata_reorder and since no
other job is submitted to padata, modprobe waits indefinitely.

Add a pair of full barriers to guarantee proper ordering:

CPU0                                 CPU1

padata_reorder                       padata_do_serial
  UNLOCK pd->lock
  smp_mb()
  LOAD reorder_objects
                                       INC reorder_objects
                                       smp_mb__after_atomic()
                                       padata_reorder
                                         TRYLOCK pd->lock

smp_mb__after_atomic is needed so the read part of the trylock operation
comes after the INC, as Andrea points out.   Thanks also to Andrea for
help with writing a litmus test.

Fixes: 16295bec6398 ("padata: Generic parallelization/serialization interface")
Signed-off-by: Daniel Jordan 
Cc: Andrea Parri 
Cc: Boqun Feng 
Cc: Herbert Xu 
Cc: Paul E. McKenney 
Cc: Peter Zijlstra 
Cc: Steffen Klassert 
Cc: linux-arch@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Herbert Xu 
Signed-off-by: Ben Hutchings

padata: free correct variable

2017-08-26T01:14:30+00:00

commit 07a77929ba672d93642a56dc2255dd21e6e2290b upstream.

The author meant to free the variable that was just allocated, instead
of the one that failed to be allocated, but made a simple typo. This
patch rectifies that.

Signed-off-by: Jason A. Donenfeld 
Signed-off-by: Herbert Xu 
Signed-off-by: Ben Hutchings

padata: Fix wrong usage of rcu_dereference()

2013-12-05T13:28:42+00:00

A kernel with enabled lockdep complains about the wrong usage of
rcu_dereference() under a rcu_read_lock_bh() protected region.

  ===============================
  [ INFO: suspicious RCU usage. ]
  3.13.0-rc1+ #126 Not tainted
  -------------------------------
  linux/kernel/padata.c:115 suspicious rcu_dereference_check() usage!

  other info that might help us debug this:

  rcu_scheduler_active = 1, debug_locks = 1
  1 lock held by cryptomgr_test/153:
   #0:  (rcu_read_lock_bh){.+....}, at: [] padata_do_parallel+0x5/0x270

Fix that by using rcu_dereference_bh() instead.

Signed-off-by: Mathias Krause 
Acked-by: Steffen Klassert 
Signed-off-by: Herbert Xu

padata: make the sequence counter an atomic_t

2013-10-30T04:02:58+00:00

Using a spinlock to atomically increase a counter sounds wrong -- we've
atomic_t for this!

Also move 'seq_nr' to a different cache line than 'lock' to reduce cache
line trashing. This has the nice side effect of decreasing the size of
struct parallel_data from 192 to 128 bytes for a x86-64 build, e.g.
occupying only two instead of three cache lines.

Those changes results in a 5% performance increase on an IPsec test run
using pcrypt.

Btw. the seq_lock spinlock was never explicitly initialized -- one more
reason to get rid of it.

Signed-off-by: Mathias Krause 
Acked-by: Steffen Klassert 
Signed-off-by: Herbert Xu

padata - Register hotcpu notifier after initialization

2013-08-29T04:37:59+00:00

padata_cpu_callback() takes pinst->lock, to avoid taking
an uninitialized lock, register the notifier after it's
initialization.

Signed-off-by: Richard Weinberger 
Acked-by: Steffen Klassert 
Signed-off-by: Herbert Xu

padata - share code between CPU_ONLINE and CPU_DOWN_FAILED, same to CPU_DOWN_PREPARE and CPU_UP_CANCELED

2013-08-29T04:37:59+00:00

Share code between CPU_ONLINE and CPU_DOWN_FAILED, same to
CPU_DOWN_PREPARE and CPU_UP_CANCELED.

It will fix 2 bugs:

  "not check the return value of __padata_remove_cpu() and __padata_add_cpu()".
  "need add 'break' between CPU_UP_CANCELED and CPU_DOWN_FAILED".

Signed-off-by: Chen Gang 
Acked-by: Steffen Klassert 
Signed-off-by: Herbert Xu

padata: use __this_cpu_read per-cpu helper

2012-12-06T09:16:23+00:00

For bottom halves off, __this_cpu_read is better.

Signed-off-by: Shan Wei 
Reviewed-by: Christoph Lameter 
Acked-by: Steffen Klassert 
Signed-off-by: Herbert Xu

padata: Fix cpu hotplug

2012-03-29T11:52:46+00:00

We don't remove the cpu that went offline from our cpumasks
on cpu hotplug. This got lost somewhere along the line, so
restore it. This fixes a hang of the padata instance on cpu
hotplug.

Signed-off-by: Steffen Klassert 
Signed-off-by: Herbert Xu

padata: Use the online cpumask as the default

2012-03-29T11:52:46+00:00

We use the active cpumask to determine the superset of cpus
to use for parallelization. However, the active cpumask is
for internal usage of the scheduler and therefore not the
appropriate cpumask for these purposes. So use the online
cpumask instead.

Reported-by: Peter Zijlstra 
Signed-off-by: Steffen Klassert 
Signed-off-by: Herbert Xu

padata: Add a reference to the api documentation

2012-03-29T11:52:45+00:00

Add a reference to the padata api documentation at Documentation/padata.txt

Suggested-by: Peter Zijlstra 
Signed-off-by: Steffen Klassert 
Signed-off-by: Herbert Xu