Patchwork Correct rtas_data_buf locking in dlpar code

login
register
mail settings
Submitter Nathan Fontenot
Date Aug. 18, 2010, 7:58 p.m.
Message ID <4C6C3B76.8040105@austin.ibm.com>
Download mbox | patch
Permalink /patch/62085/
State Accepted, archived
Commit 93f68f1ef787d97ab688f78a01f446e85bb9a496
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Nathan Fontenot - Aug. 18, 2010, 7:58 p.m.
The dlpar code can cause a deadlock to occur when making the RTAS
configure-connector call.  This occurs because we make kmalloc calls,
which can block, while parsing the rtas_data_buf and holding the
rtas_data_buf_lock.  This an cause issues if someone else attempts
to grab the rtas_data_bug_lock.

This patch alleviates this issue by copying the contents of the rtas_data_buf
to a local buffer before parsing.  This allows us to only hold the
rtas_data_buf_lock around the RTAS configure-connector calls.

Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>

---
 arch/powerpc/platforms/pseries/dlpar.c |   42 ++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 13 deletions(-)

Patch

Index: powerpc/arch/powerpc/platforms/pseries/dlpar.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/pseries/dlpar.c	2010-08-05 10:55:46.000000000 -0500
+++ powerpc/arch/powerpc/platforms/pseries/dlpar.c	2010-08-18 11:42:10.000000000 -0500
@@ -129,20 +129,35 @@  struct device_node *dlpar_configure_conn
 	struct property *property;
 	struct property *last_property = NULL;
 	struct cc_workarea *ccwa;
+	char *data_buf;
 	int cc_token;
-	int rc;
+	int rc = -1;
 
 	cc_token = rtas_token("ibm,configure-connector");
 	if (cc_token == RTAS_UNKNOWN_SERVICE)
 		return NULL;
 
-	spin_lock(&rtas_data_buf_lock);
-	ccwa = (struct cc_workarea *)&rtas_data_buf[0];
+	data_buf = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
+	if (!data_buf)
+		return NULL;
+
+	ccwa = (struct cc_workarea *)&data_buf[0];
 	ccwa->drc_index = drc_index;
 	ccwa->zero = 0;
 
-	rc = rtas_call(cc_token, 2, 1, NULL, rtas_data_buf, NULL);
-	while (rc) {
+	do {
+		/* Since we release the rtas_data_buf lock between configure
+		 * connector calls we want to re-populate the rtas_data_buffer
+		 * with the contents of the previous call.
+		 */
+		spin_lock(&rtas_data_buf_lock);
+
+		memcpy(rtas_data_buf, data_buf, RTAS_DATA_BUF_SIZE);
+		rc = rtas_call(cc_token, 2, 1, NULL, rtas_data_buf, NULL);
+		memcpy(data_buf, rtas_data_buf, RTAS_DATA_BUF_SIZE);
+
+		spin_unlock(&rtas_data_buf_lock);
+
 		switch (rc) {
 		case NEXT_SIBLING:
 			dn = dlpar_parse_cc_node(ccwa);
@@ -197,18 +212,19 @@  struct device_node *dlpar_configure_conn
 			       "returned from configure-connector\n", rc);
 			goto cc_error;
 		}
+	} while (rc);
 
-		rc = rtas_call(cc_token, 2, 1, NULL, rtas_data_buf, NULL);
+cc_error:
+	kfree(data_buf);
+
+	if (rc) {
+		if (first_dn)
+			dlpar_free_cc_nodes(first_dn);
+
+		return NULL;
 	}
 
-	spin_unlock(&rtas_data_buf_lock);
 	return first_dn;
-
-cc_error:
-	if (first_dn)
-		dlpar_free_cc_nodes(first_dn);
-	spin_unlock(&rtas_data_buf_lock);
-	return NULL;
 }
 
 static struct device_node *derive_parent(const char *path)