Patch "Revert "module, async: async_synchronize_full() on module init iff async is used"" has been added to the 5.16-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    Revert "module, async: async_synchronize_full() on module init iff async is used"

to the 5.16-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     revert-module-async-async_synchronize_full-on-module.patch
and it can be found in the queue-5.16 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 6130034213bc19289111295eda2beb193d28e254
Author: Igor Pylypiv <ipylypiv@xxxxxxxxxx>
Date:   Thu Jan 27 15:39:53 2022 -0800

    Revert "module, async: async_synchronize_full() on module init iff async is used"
    
    [ Upstream commit 67d6212afda218d564890d1674bab28e8612170f ]
    
    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>
    Reviewed-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
    Acked-by: Tejun Heo <tj@xxxxxxxxxx>
    Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 78c351e35fec6..ee5ed88219631 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -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 */
diff --git a/kernel/async.c b/kernel/async.c
index b8d7a663497f9..b2c4ba5686ee4 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -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);
 
diff --git a/kernel/module.c b/kernel/module.c
index 84a9141a5e159..f25e7653aa150 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -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)
@@ -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 +



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux