From: Tomas Jedlicka <tomas.jedlicka@oracle.com>
Date: Tue, 25 Jul 2017 09:54:48 +0000 (-0400)
Subject: dtrace: should not sleep in idr code paths
X-Git-Tag: v4.1.12-111.0.20170918_2215~182^2~1
X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=508457ee31f5a123e9a720104b6e6add6e85612a;p=users%2Fjedix%2Flinux-maple.git

dtrace: should not sleep in idr code paths

The idr_preload() causes thread to continue in atomic context.
Taking mutex in this code path may lead to deadlock or scheduler
problems.  This fix alters locking scheme in a way that may cause
some latency issues in DTrace framework but will keep host safe.

Orabug: 26680802

Signed-off-by: Tomas Jedlicka <tomas.jedlicka@oracle.com>
Reviewed-by: Nick Alcock <nick.alcock@oracle.com>
---

diff --git a/dtrace/dtrace_ecb.c b/dtrace/dtrace_ecb.c
index 52b04badc3f03..4e551a4ea318c 100644
--- a/dtrace/dtrace_ecb.c
+++ b/dtrace/dtrace_ecb.c
@@ -164,11 +164,7 @@ success:
 	/*
 	 * Get an ID for the aggregation (add it to the idr).
 	 */
-	mutex_unlock(&dtrace_lock);
-
 	idr_preload(GFP_KERNEL);
-	mutex_lock(&dtrace_lock);
-
 	aggid = idr_alloc_cyclic(&state->dts_agg_idr, agg, 0, 0, GFP_NOWAIT);
 	idr_preload_end();
 	if (aggid < 0) {
diff --git a/dtrace/dtrace_probe.c b/dtrace/dtrace_probe.c
index de6e6edc34189..46f7db1294d75 100644
--- a/dtrace/dtrace_probe.c
+++ b/dtrace/dtrace_probe.c
@@ -53,18 +53,24 @@ dtrace_id_t dtrace_probe_create(dtrace_provider_id_t prov, const char *mod,
 	probe = kmem_cache_alloc(dtrace_probe_cachep, __GFP_NOFAIL);
 
 	/*
-	 * The ir_preload() function should be called without holding locks.
+	 * The idr_preload() should be called without holding locks as it may
+	 * block.  At the same time it is required to protect DTrace structures.
+	 * We can't drop it before idr_preload() and acquire after it because
+	 * we can't sleep in atomic context (until we reach idr_preload_end()).
+	 *
+	 * It is better to delay DTrace framework than traced host so the lock
+	 * is being held for the duration of idr allocation.
+	 *
 	 * When the provider is the DTrace core itself, dtrace_lock will be
 	 * held when we enter this function.
 	 */
 	if (provider == dtrace_provider) {
 		ASSERT(MUTEX_HELD(&dtrace_lock));
-		mutex_unlock(&dtrace_lock);
+	} else {
+		mutex_lock(&dtrace_lock);
 	}
 
 	idr_preload(GFP_KERNEL);
-
-	mutex_lock(&dtrace_lock);
 	id = idr_alloc_cyclic(&dtrace_probe_idr, probe, 0, 0, GFP_NOWAIT);
 	idr_preload_end();
 	if (id < 0) {
@@ -1282,16 +1288,7 @@ int dtrace_probe_init(void)
 	 * We need to drop our locks when calling idr_preload(), so we try to
 	 * get them back right after.
 	 */
-	mutex_unlock(&dtrace_lock);
-	mutex_unlock(&dtrace_provider_lock);
-	mutex_unlock(&cpu_lock);
-
 	idr_preload(GFP_KERNEL);
-
-	mutex_lock(&cpu_lock);
-	mutex_lock(&dtrace_provider_lock);
-	mutex_lock(&dtrace_lock);
-
 	id = idr_alloc_cyclic(&dtrace_probe_idr, NULL, 0, 0, GFP_NOWAIT);
 	idr_preload_end();
 
diff --git a/dtrace/dtrace_state.c b/dtrace/dtrace_state.c
index 249642965a232..c04be314f8eb2 100644
--- a/dtrace/dtrace_state.c
+++ b/dtrace/dtrace_state.c
@@ -352,14 +352,7 @@ dtrace_state_t *dtrace_state_create(struct file *file)
 	 * Create a first entry in the aggregation IDR, so that ID 0 is used as
 	 * that gets used as meaning 'none'.
 	 */
-	mutex_unlock(&dtrace_lock);
-	mutex_unlock(&cpu_lock);
-
 	idr_preload(GFP_KERNEL);
-
-	mutex_lock(&cpu_lock);
-	mutex_lock(&dtrace_lock);
-
 	aggid = idr_alloc_cyclic(&state->dts_agg_idr, NULL, 0, 0, GFP_NOWAIT);
 	idr_preload_end();