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 <redacted>
Reviewed-by: Changyuan Lyu <redacted>

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@kernel.org>

Thanks.

-- 
tejun