linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] random: Support freezable kthreads in add_hwgenerator_randomness()
@ 2019-08-19 15:02 Stephen Boyd
  2019-08-22  5:55 ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2019-08-19 15:02 UTC (permalink / raw)
  To: Herbert Xu
  Cc: linux-kernel, Theodore Ts'o, linux-crypto, Matt Mackall, Keerthy

The kthread calling this function is freezable after commit 03a3bb7ae631
("hwrng: core - Freeze khwrng thread during suspend") is applied.
Unfortunately, this function uses wait_event_interruptible() but doesn't
check for the kthread being woken up by the fake freezer signal. When a
user suspends the system, this kthread will wake up and if it fails the
entropy size check it will immediately go back to sleep and not go into
the freezer. Eventually, suspend will fail because the task never froze
and a warning message like this may appear:

 PM: suspend entry (deep)
 Filesystems sync: 0.000 seconds
 Freezing user space processes ... (elapsed 0.001 seconds) done.
 OOM killer disabled.
 Freezing remaining freezable tasks ...
 Freezing of tasks failed after 20.003 seconds (1 tasks refusing to freeze, wq_busy=0):
 hwrng           R  running task        0   289      2 0x00000020
 [<c08c64c4>] (__schedule) from [<c08c6a10>] (schedule+0x3c/0xc0)
 [<c08c6a10>] (schedule) from [<c05dbd8c>] (add_hwgenerator_randomness+0xb0/0x100)
 [<c05dbd8c>] (add_hwgenerator_randomness) from [<bf1803c8>] (hwrng_fillfn+0xc0/0x14c [rng_core])
 [<bf1803c8>] (hwrng_fillfn [rng_core]) from [<c015abec>] (kthread+0x134/0x148)
 [<c015abec>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)

Check for a freezer signal here and skip adding any randomness if the
task wakes up because it was frozen. This should make the kthread freeze
properly and suspend work again.

Fixes: 03a3bb7ae631 ("hwrng: core - Freeze khwrng thread during suspend")
Reported-by: Keerthy <j-keerthy@ti.com>
Tested-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

Probably needs to go via Herbert who routed the patch this is fixing.

 drivers/char/random.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 5d5ea4ce1442..e2e85ca16410 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2429,6 +2429,7 @@ void add_hwgenerator_randomness(const char *buffer, size_t count,
 				size_t entropy)
 {
 	struct entropy_store *poolp = &input_pool;
+	bool frozen = false;
 
 	if (unlikely(crng_init == 0)) {
 		crng_fast_load(buffer, count);
@@ -2439,9 +2440,12 @@ void add_hwgenerator_randomness(const char *buffer, size_t count,
 	 * We'll be woken up again once below random_write_wakeup_thresh,
 	 * or when the calling thread is about to terminate.
 	 */
-	wait_event_interruptible(random_write_wait, kthread_should_stop() ||
+	wait_event_interruptible(random_write_wait,
+			kthread_freezable_should_stop(&frozen) ||
 			ENTROPY_BITS(&input_pool) <= random_write_wakeup_bits);
-	mix_pool_bytes(poolp, buffer, count);
-	credit_entropy_bits(poolp, entropy);
+	if (!frozen) {
+		mix_pool_bytes(poolp, buffer, count);
+		credit_entropy_bits(poolp, entropy);
+	}
 }
 EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
-- 
Sent by a computer through tubes


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

* Re: [PATCH] random: Support freezable kthreads in add_hwgenerator_randomness()
  2019-08-19 15:02 [PATCH] random: Support freezable kthreads in add_hwgenerator_randomness() Stephen Boyd
@ 2019-08-22  5:55 ` Herbert Xu
  2019-09-04 11:00   ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2019-08-22  5:55 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-kernel, Theodore Ts'o, linux-crypto, Matt Mackall, Keerthy

On Mon, Aug 19, 2019 at 08:02:45AM -0700, Stephen Boyd wrote:
> The kthread calling this function is freezable after commit 03a3bb7ae631
> ("hwrng: core - Freeze khwrng thread during suspend") is applied.
> Unfortunately, this function uses wait_event_interruptible() but doesn't
> check for the kthread being woken up by the fake freezer signal. When a
> user suspends the system, this kthread will wake up and if it fails the
> entropy size check it will immediately go back to sleep and not go into
> the freezer. Eventually, suspend will fail because the task never froze
> and a warning message like this may appear:
> 
>  PM: suspend entry (deep)
>  Filesystems sync: 0.000 seconds
>  Freezing user space processes ... (elapsed 0.001 seconds) done.
>  OOM killer disabled.
>  Freezing remaining freezable tasks ...
>  Freezing of tasks failed after 20.003 seconds (1 tasks refusing to freeze, wq_busy=0):
>  hwrng           R  running task        0   289      2 0x00000020
>  [<c08c64c4>] (__schedule) from [<c08c6a10>] (schedule+0x3c/0xc0)
>  [<c08c6a10>] (schedule) from [<c05dbd8c>] (add_hwgenerator_randomness+0xb0/0x100)
>  [<c05dbd8c>] (add_hwgenerator_randomness) from [<bf1803c8>] (hwrng_fillfn+0xc0/0x14c [rng_core])
>  [<bf1803c8>] (hwrng_fillfn [rng_core]) from [<c015abec>] (kthread+0x134/0x148)
>  [<c015abec>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
> 
> Check for a freezer signal here and skip adding any randomness if the
> task wakes up because it was frozen. This should make the kthread freeze
> properly and suspend work again.
> 
> Fixes: 03a3bb7ae631 ("hwrng: core - Freeze khwrng thread during suspend")
> Reported-by: Keerthy <j-keerthy@ti.com>
> Tested-by: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> 
> Probably needs to go via Herbert who routed the patch this is fixing.
> 
>  drivers/char/random.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: [PATCH] random: Support freezable kthreads in add_hwgenerator_randomness()
  2019-08-22  5:55 ` Herbert Xu
@ 2019-09-04 11:00   ` Sebastian Andrzej Siewior
  2019-09-04 18:49     ` Stephen Boyd
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2019-09-04 11:00 UTC (permalink / raw)
  To: Herbert Xu, Stephen Boyd
  Cc: linux-kernel, Theodore Ts'o, linux-crypto, Matt Mackall,
	Keerthy, Thomas Gleixner, Peter Zijlstra

On 2019-08-22 15:55:19 [+1000], Herbert Xu wrote:
> Patch applied.  Thanks.
[ ff296293b3538 ("random: Support freezable kthreads in add_hwgenerator_randomness()") ]

and since kthread_freezable_should_stop() has might_sleep() in it, I get
this:

|: do not call blocking ops when !TASK_RUNNING; state=1 set at [<00000000349d1489>] prepare_to_wait_event+0x5a/0x180
|: WARNING: CPU: 0 PID: 828 at kernel/sched/core.c:6741 __might_sleep+0x6f/0x80
|: Modules linked in:
|:
|: CPU: 0 PID: 828 Comm: hwrng Not tainted 5.3.0-rc7-next-20190903+ #46
|: RIP: 0010:__might_sleep+0x6f/0x80
…
|: Call Trace:
|:  kthread_freezable_should_stop+0x1b/0x60
|:  add_hwgenerator_randomness+0xdd/0x130
|:  hwrng_fillfn+0xbf/0x120
|:  kthread+0x10c/0x140
|:  ret_from_fork+0x27/0x50

Sebastian

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

* Re: [PATCH] random: Support freezable kthreads in add_hwgenerator_randomness()
  2019-09-04 11:00   ` Sebastian Andrzej Siewior
@ 2019-09-04 18:49     ` Stephen Boyd
  2019-09-05  7:41       ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2019-09-04 18:49 UTC (permalink / raw)
  To: Herbert Xu, Sebastian Andrzej Siewior
  Cc: linux-kernel, Theodore Ts'o, linux-crypto, Matt Mackall,
	Keerthy, Thomas Gleixner, Peter Zijlstra

Quoting Sebastian Andrzej Siewior (2019-09-04 04:00:38)
> On 2019-08-22 15:55:19 [+1000], Herbert Xu wrote:
> > Patch applied.  Thanks.
> [ ff296293b3538 ("random: Support freezable kthreads in add_hwgenerator_randomness()") ]
> 
> and since kthread_freezable_should_stop() has might_sleep() in it, I get
> this:
> 
> |: do not call blocking ops when !TASK_RUNNING; state=1 set at [<00000000349d1489>] prepare_to_wait_event+0x5a/0x180
> |: WARNING: CPU: 0 PID: 828 at kernel/sched/core.c:6741 __might_sleep+0x6f/0x80
> |: Modules linked in:
> |:
> |: CPU: 0 PID: 828 Comm: hwrng Not tainted 5.3.0-rc7-next-20190903+ #46
> |: RIP: 0010:__might_sleep+0x6f/0x80
> …
> |: Call Trace:
> |:  kthread_freezable_should_stop+0x1b/0x60
> |:  add_hwgenerator_randomness+0xdd/0x130
> |:  hwrng_fillfn+0xbf/0x120
> |:  kthread+0x10c/0x140
> |:  ret_from_fork+0x27/0x50
> 

Ugh ok. Thanks for the report.

We're getting warnings because the task is in TASK_INTERRUPTIBLE state
when we call kthread_freezable_should_stop() from deep within the wait
event code. We shouldn't do that, and instead we should call
wait_event_freezable() and kthread_should_stop() in the condition. This
way we'll call into the freezer when the task is woken up by the suspend
path.

Can you try this?

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 9b54cdb301d3..d3beed084c0a 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -327,6 +327,7 @@
 #include <linux/percpu.h>
 #include <linux/cryptohash.h>
 #include <linux/fips.h>
+#include <linux/freezer.h>
 #include <linux/ptrace.h>
 #include <linux/workqueue.h>
 #include <linux/irq.h>
@@ -2429,7 +2430,6 @@ void add_hwgenerator_randomness(const char *buffer, size_t count,
 				size_t entropy)
 {
 	struct entropy_store *poolp = &input_pool;
-	bool frozen = false;
 
 	if (unlikely(crng_init == 0)) {
 		crng_fast_load(buffer, count);
@@ -2440,13 +2440,11 @@ void add_hwgenerator_randomness(const char *buffer, size_t count,
 	 * We'll be woken up again once below random_write_wakeup_thresh,
 	 * or when the calling thread is about to terminate.
 	 */
-	wait_event_interruptible(random_write_wait,
-			kthread_freezable_should_stop(&frozen) ||
+	wait_event_freezable(random_write_wait,
+			kthread_should_stop() ||
 			ENTROPY_BITS(&input_pool) <= random_write_wakeup_bits);
-	if (!frozen) {
-		mix_pool_bytes(poolp, buffer, count);
-		credit_entropy_bits(poolp, entropy);
-	}
+	mix_pool_bytes(poolp, buffer, count);
+	credit_entropy_bits(poolp, entropy);
 }
 EXPORT_SYMBOL_GPL(add_hwgenerator_randomness);
 


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

* Re: [PATCH] random: Support freezable kthreads in add_hwgenerator_randomness()
  2019-09-04 18:49     ` Stephen Boyd
@ 2019-09-05  7:41       ` Sebastian Andrzej Siewior
  2019-09-05 16:31         ` Stephen Boyd
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2019-09-05  7:41 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Herbert Xu, linux-kernel, Theodore Ts'o, linux-crypto,
	Matt Mackall, Keerthy, Thomas Gleixner, Peter Zijlstra

On 2019-09-04 11:49:57 [-0700], Stephen Boyd wrote:
> Can you try this?

yes, works.

Sebastian

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

* Re: [PATCH] random: Support freezable kthreads in add_hwgenerator_randomness()
  2019-09-05  7:41       ` Sebastian Andrzej Siewior
@ 2019-09-05 16:31         ` Stephen Boyd
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2019-09-05 16:31 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Herbert Xu, linux-kernel, Theodore Ts'o, linux-crypto,
	Matt Mackall, Keerthy, Thomas Gleixner, Peter Zijlstra

Quoting Sebastian Andrzej Siewior (2019-09-05 00:41:52)
> On 2019-09-04 11:49:57 [-0700], Stephen Boyd wrote:
> > Can you try this?
> 
> yes, works.
> 

Cool thanks. I'll send a proper patch with your tested-by then?
Alternatively this can be squashed into this previous patch because it
was all wrong and is basically reverting the patch and changing the
wait_event call to be freezable.


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

end of thread, other threads:[~2019-09-05 16:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 15:02 [PATCH] random: Support freezable kthreads in add_hwgenerator_randomness() Stephen Boyd
2019-08-22  5:55 ` Herbert Xu
2019-09-04 11:00   ` Sebastian Andrzej Siewior
2019-09-04 18:49     ` Stephen Boyd
2019-09-05  7:41       ` Sebastian Andrzej Siewior
2019-09-05 16:31         ` Stephen Boyd

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).