On Wed, Aug 31, 2022 at 05:20:24PM +0000, Sven van Ashbrook wrote: > The hwrng fill function runs as a normal kthread. This thread > doesn't get frozen by the PM, i.e. it will keep running during, > or in, system suspend. It may call the client driver's > data_present()/data_read() functions during, or in, suspend; > which may generate errors or warnings. For example, if the > client driver uses an i2c bus, the following warning may be > intermittently generated: > > i2c: Transfer while suspended > > Fix by converting the delay polled kthread into an ordered work > queue running a single, self-rearming delayed_work. Make the > workqueue WQ_FREEZABLE, so the PM will drain any work items > before going into suspend. This prevents client drivers from > being accessed during, or in, suspend. > > Tested on a Chromebook containing an cr50 tpm over i2c. The test > consists of 31000 suspend/resume cycles. Occasional > "i2c: Transfer while suspended" warnings are seen. After applying > this patch, these warnings disappear. > > This patch also does not appear to cause any regressions on the > ChromeOS test queues. > > Signed-off-by: Sven van Ashbrook <svenva@xxxxxxxxxxxx> The general concept seems to be fine. > - if (rc <= 0) { > - pr_warn("hwrng: no data available\n"); > - msleep_interruptible(10000); > - continue; > - } > + if (!quality) > + return; > > + if (rc > 0) { > /* If we cannot credit at least one bit of entropy, > * keep track of the remainder for the next iteration > */ You need to refresh your patch-set against the latest tree. This function has changed quite a bit. > @@ -541,14 +536,12 @@ static void hwrng_manage_rngd(struct hwrng *rng) > if (WARN_ON(!mutex_is_locked(&rng_mutex))) > return; > > - if (rng->quality == 0 && hwrng_fill) > - kthread_stop(hwrng_fill); > - if (rng->quality > 0 && !hwrng_fill) { > - hwrng_fill = kthread_run(hwrng_fillfn, NULL, "hwrng"); > - if (IS_ERR(hwrng_fill)) { > - pr_err("hwrng_fill thread creation failed\n"); > - hwrng_fill = NULL; > - } > + if (rng->quality == 0 && is_hwrng_wq_running) { > + cancel_delayed_work(&hwrng_fill_dwork); > + is_hwrng_wq_running = false; > + } else if (rng->quality > 0 && !is_hwrng_wq_running) { > + mod_delayed_work(hwrng_wq, &hwrng_fill_dwork, 0); > + is_hwrng_wq_running = true; > } > } > > @@ -631,8 +624,7 @@ void hwrng_unregister(struct hwrng *rng) > new_rng = get_current_rng_nolock(); > if (list_empty(&rng_list)) { > mutex_unlock(&rng_mutex); > - if (hwrng_fill) > - kthread_stop(hwrng_fill); > + cancel_delayed_work_sync(&hwrng_fill_dwork); What if hwrng_manage_rngd races against hwrng_unregister? Thanks, -- Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt