| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20220106004656.126790-5-daniel.m.jordan@oracle.com>
Date: Wed, 5 Jan 2022 19:46:44 -0500
From: Daniel Jordan <daniel.m.jordan@...cle.com>
To: Alexander Duyck <alexanderduyck@...com>,
Alex Williamson <alex.williamson@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Ben Segall <bsegall@...gle.com>,
Cornelia Huck <cohuck@...hat.com>,
Dan Williams <dan.j.williams@...el.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Herbert Xu <herbert@...dor.apana.org.au>,
Ingo Molnar <mingo@...hat.com>,
Jason Gunthorpe <jgg@...dia.com>,
Johannes Weiner <hannes@...xchg.org>,
Josh Triplett <josh@...htriplett.org>,
Michal Hocko <mhocko@...e.com>, Nico Pache <npache@...hat.com>,
Pasha Tatashin <pasha.tatashin@...een.com>,
Peter Zijlstra <peterz@...radead.org>,
Steffen Klassert <steffen.klassert@...unet.com>,
Steve Sistare <steven.sistare@...cle.com>,
Tejun Heo <tj@...nel.org>,
Tim Chen <tim.c.chen@...ux.intel.com>,
Vincent Guittot <vincent.guittot@...aro.org>
Cc: linux-mm@...ck.org, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
Daniel Jordan <daniel.m.jordan@...cle.com>
Subject: [RFC 04/16] padata: Detect deadlocks between main and helper threads
A caller of padata_do_multithreaded() can unwittingly introduce
deadlocks if it already holds lock(s) that thread_fn() takes. Lockdep
can't detect such a dependency because it doesn't know that
padata_do_multithreaded() waits on the helper threads.
Use a lockdep_map to encode the dependency, following the pattern in
workqueue, CPU hotplug, and other parts of the kernel. See
commit 4e6045f13478 ("workqueue: debug flushing deadlocks with lockdep")
for an example of a similar situation.
Each padata_do_multithreaded() callsite gets its own lock_class_key to
avoid false positives involving locks from different calls that don't
depend on each other.
Signed-off-by: Daniel Jordan <daniel.m.jordan@...cle.com>
Suggested-by: Peter Zijlstra <peterz@...radead.org>
---
include/linux/padata.h | 22 +++++++++++++++++++++-
kernel/padata.c | 15 +++++++++++++--
2 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/include/linux/padata.h b/include/linux/padata.h
index 2a9fa459463d..907d624a8ca4 100644
--- a/include/linux/padata.h
+++ b/include/linux/padata.h
@@ -17,6 +17,7 @@
#include <linux/spinlock.h>
#include <linux/list.h>
#include <linux/kobject.h>
+#include <linux/lockdep.h>
#define PADATA_CPU_SERIAL 0x01
#define PADATA_CPU_PARALLEL 0x02
@@ -188,6 +189,23 @@ extern void __init padata_init(void);
static inline void __init padata_init(void) {}
#endif
+#ifdef CONFIG_LOCKDEP
+
+#define padata_do_multithreaded(job) \
+({ \
+ static struct lock_class_key __key; \
+ const char *__map_name = "padata master waiting"; \
+ \
+ padata_do_multithreaded_job((job), &__key, __map_name); \
+})
+
+#else
+
+#define padata_do_multithreaded(job) \
+ padata_do_multithreaded_job((job), NULL, NULL)
+
+#endif
+
extern struct padata_instance *padata_alloc(const char *name);
extern void padata_free(struct padata_instance *pinst);
extern struct padata_shell *padata_alloc_shell(struct padata_instance *pinst);
@@ -195,7 +213,9 @@ extern void padata_free_shell(struct padata_shell *ps);
extern int padata_do_parallel(struct padata_shell *ps,
struct padata_priv *padata, int *cb_cpu);
extern void padata_do_serial(struct padata_priv *padata);
-extern int padata_do_multithreaded(struct padata_mt_job *job);
+extern int padata_do_multithreaded_job(struct padata_mt_job *job,
+ struct lock_class_key *key,
+ const char *map_name);
extern int padata_set_cpumask(struct padata_instance *pinst, int cpumask_type,
cpumask_var_t cpumask);
#endif
diff --git a/kernel/padata.c b/kernel/padata.c
index d0876f861464..b458deb17121 100644
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -64,6 +64,9 @@ struct padata_mt_job_state {
unsigned long position;
unsigned long remaining_size;
struct list_head failed_works;
+#ifdef CONFIG_LOCKDEP
+ struct lockdep_map lockdep_map;
+#endif
};
static void padata_free_pd(struct parallel_data *pd);
@@ -470,9 +473,11 @@ static void padata_mt_helper(struct work_struct *w)
ps->remaining_size -= size;
spin_unlock(&ps->lock);
+ lock_map_acquire(&ps->lockdep_map);
ret = job->thread_fn(position, end, job->fn_arg);
+ lock_map_release(&ps->lockdep_map);
spin_lock(&ps->lock);
if (ret) {
@@ -552,14 +557,16 @@ static void padata_undo(struct padata_mt_job_state *ps,
}
/**
- * padata_do_multithreaded - run a multithreaded job
+ * padata_do_multithreaded_job - run a multithreaded job
* @job: Description of the job.
*
* See the definition of struct padata_mt_job for more details.
*
* Return: 0 or a client-specific nonzero error code.
*/
-int padata_do_multithreaded(struct padata_mt_job *job)
+int padata_do_multithreaded_job(struct padata_mt_job *job,
+ struct lock_class_key *key,
+ const char *map_name)
{
/* In case threads finish at different times. */
static const unsigned long load_balance_factor = 4;
@@ -583,6 +590,7 @@ int padata_do_multithreaded(struct padata_mt_job *job)
spin_lock_init(&ps.lock);
init_completion(&ps.completion);
+ lockdep_init_map(&ps.lockdep_map, map_name, key, 0);
INIT_LIST_HEAD(&ps.failed_works);
ps.job = job;
ps.nworks = padata_work_alloc_mt(nworks, &ps, &works);
@@ -601,6 +609,9 @@ int padata_do_multithreaded(struct padata_mt_job *job)
ps.chunk_size = max(ps.chunk_size, job->min_chunk);
ps.chunk_size = roundup(ps.chunk_size, job->align);
+ lock_map_acquire(&ps.lockdep_map);
+ lock_map_release(&ps.lockdep_map);
+
list_for_each_entry(pw, &works, pw_list)
queue_work(system_unbound_wq, &pw->pw_work);
--
2.34.1
Powered by blists - more mailing lists