Skip to content

Commit

Permalink
Revert "module, async: async_synchronize_full() on module init iff as…
Browse files Browse the repository at this point in the history
…ync is used"

This reverts commit 774a122.

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 21c3c5d ("block: don't request module during elevator init")
fixed the deadlock issue which the reverted commit 774a122
("module, async: async_synchronize_full() on module init iff async is
used") tried to fix.

Since commit 0fdff3e ("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@google.com>
Reviewed-by: Changyuan Lyu <changyuanl@google.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Igor Pylypiv authored and Linus Torvalds committed Feb 3, 2022
1 parent 305e6c4 commit 67d6212
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 24 deletions.
1 change: 0 additions & 1 deletion include/linux/sched.h
Original file line number Diff line number Diff line change
Expand Up @@ -1680,7 +1680,6 @@ extern struct pid *cad_pid;
#define PF_MEMALLOC 0x00000800 /* Allocating memory */
#define PF_NPROC_EXCEEDED 0x00001000 /* set_user() noticed that RLIMIT_NPROC was exceeded */
#define PF_USED_MATH 0x00002000 /* If unset the fpu must be initialized before use */
#define PF_USED_ASYNC 0x00004000 /* Used async_schedule*(), used by module init */
#define PF_NOFREEZE 0x00008000 /* This thread should not be frozen */
#define PF_FROZEN 0x00010000 /* Frozen for system suspend */
#define PF_KSWAPD 0x00020000 /* I am kswapd */
Expand Down
3 changes: 0 additions & 3 deletions kernel/async.c
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,6 @@ async_cookie_t async_schedule_node_domain(async_func_t func, void *data,
atomic_inc(&entry_count);
spin_unlock_irqrestore(&async_lock, flags);

/* mark that this task has queued an async job, used by module init */
current->flags |= PF_USED_ASYNC;

/* schedule for execution */
queue_work_node(node, system_unbound_wq, &entry->work);

Expand Down
25 changes: 5 additions & 20 deletions kernel/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -3725,12 +3725,6 @@ static noinline int do_init_module(struct module *mod)
}
freeinit->module_init = mod->init_layout.base;

/*
* We want to find out whether @mod uses async during init. Clear
* PF_USED_ASYNC. async_schedule*() will set it.
*/
current->flags &= ~PF_USED_ASYNC;

do_mod_ctors(mod);
/* Start the module */
if (mod->init != NULL)
Expand All @@ -3756,22 +3750,13 @@ static noinline int do_init_module(struct module *mod)

/*
* We need to finish all async code before the module init sequence
* is done. This has potential to deadlock. For example, a newly
* detected block device can trigger request_module() of the
* default iosched from async probing task. Once userland helper
* reaches here, async_synchronize_full() will wait on the async
* task waiting on request_module() and deadlock.
*
* This deadlock is avoided by perfomring async_synchronize_full()
* iff module init queued any async jobs. This isn't a full
* solution as it will deadlock the same if module loading from
* async jobs nests more than once; however, due to the various
* constraints, this hack seems to be the best option for now.
* Please refer to the following thread for details.
* is done. This has potential to deadlock if synchronous module
* loading is requested from async (which is not allowed!).
*
* http://thread.gmane.org/gmane.linux.kernel/1420814
* See commit 0fdff3ec6d87 ("async, kmod: warn on synchronous
* request_module() from async workers") for more details.
*/
if (!mod->async_probe_requested && (current->flags & PF_USED_ASYNC))
if (!mod->async_probe_requested)
async_synchronize_full();

ftrace_free_mem(mod, mod->init_layout.base, mod->init_layout.base +
Expand Down

0 comments on commit 67d6212

Please sign in to comment.