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"

[ Upstream commit 67d6212 ]

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>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
ipylypiv authored and gregkh committed Feb 23, 2022
1 parent 673894a commit a715969
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
Expand Up @@ -1672,7 +1672,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
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
Expand Up @@ -3722,12 +3722,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 @@ -3753,22 +3747,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 a715969

Please sign in to comment.