On Thu, Jan 27, 2022 at 03:39:53PM -0800, Igor Pylypiv wrote:
> This reverts commit 774a1221e862b343388347bac9b318767336b20b.
>
> We need to finish all async code before the module init sequence is done.
> In the reverted commit the PF_USED_ASYNC flag was added to mark a thread
> that called async_schedule(). Then the PF_USED_ASYNC flag was used to
> determine whether or not async_synchronize_full() needs to be invoked.
> This works when modprobe thread is calling async_schedule(), but it
> does not work if module dispatches init code to a worker thread which
> then calls async_schedule().
>
> For example, PCI driver probing is invoked from a worker thread based on
> a node where device is attached:
>
> if (cpu < nr_cpu_ids)
> error = work_on_cpu(cpu, local_pci_probe, &ddi);
> else
> error = local_pci_probe(&ddi);
>
> We end up in a situation where a worker thread gets the PF_USED_ASYNC flag
> set instead of the modprobe thread. As a result, async_synchronize_full()
> is not invoked and modprobe completes without waiting for the async code
> to finish.
>
> The issue was discovered while loading the pm80xx driver:
> (scsi_mod.scan=async)
>
> modprobe pm80xx worker
> ...
> do_init_module()
> ...
> pci_call_probe()
> work_on_cpu(local_pci_probe)
> local_pci_probe()
> pm8001_pci_probe()
> scsi_scan_host()
> async_schedule()
> worker->flags |= PF_USED_ASYNC;
> ...
> < return from worker >
> ...
> if (current->flags & PF_USED_ASYNC) <--- false
> async_synchronize_full();
>
> Commit 21c3c5d28007 ("block: don't request module during elevator init")
> fixed the deadlock issue which the reverted commit 774a1221e862 ("module,
> async: async_synchronize_full() on module init iff async is used") tried
> to fix.
>
> Since commit 0fdff3ec6d87 ("async, kmod: warn on synchronous
> request_module() from async workers") synchronous module loading
> from async is not allowed.
>
> Given that the original deadlock issue is fixed and it is no longer allowed
> to call synchronous request_module() from async we can remove PF_USED_ASYNC
> flag to make module init consistently invoke async_synchronize_full()
> unless async module probe is requested.
>
> Signed-off-by: Igor Pylypiv <ipylypiv@xxxxxxxxxx>
> Reviewed-by: Changyuan Lyu <changyuanl@xxxxxxxxxx>
That's quite a walk down the memory lane and I agree with your analysis. The
PF_USED_ASYNC is redundant for correctness with the removal of synchrnous
loading from iosched path and the WARN_ON guarantees that nothing in kernel
is creating a similar situation.
Acked-by: Tejun Heo <tj@xxxxxxxxxx>
Thanks.
--
tejun
