diff mbox

[v2,2/11] Add PRRN Event Handler

Message ID 51509CF0.10200@linux.vnet.ibm.com (mailing list archive)
State Changes Requested, archived
Delegated to: Michael Ellerman
Headers show

Commit Message

Nathan Fontenot March 25, 2013, 6:52 p.m. UTC 
From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>

A PRRN event is signaled via the RTAS event-scan mechanism, which
returns a Hot Plug Event message "fixed part" indicating "Platform
Resource Reassignment". In response to the Hot Plug Event message,
we must call ibm,update-nodes to determine which resources were
reassigned and then ibm,update-properties to obtain the new affinity
information about those resources.

The PRRN event-scan RTAS message contains only the "fixed part" with
the "Type" field set to the value 160 and no Extended Event Log. The
four-byte Extended Event Log Length field is repurposed (since no
Extended Event Log message is included) to pass the "scope" parameter
that causes the ibm,update-nodes to return the nodes affected by the
specific resource reassignment.

This patch adds a handler in rtasd for PRRN RTAS events. The function
pseries_devicetree_update() (from mobility.c) is used to make the
ibm,update-nodes/ibm,update-properties RTAS calls. Updating the NUMA maps
(handled by a subsequent patch) will require significant processing,
so pseries_devicetree_update() is called from an asynchronous workqueue
to allow rtasd to continue processing events. Since we flush all work
on the queue before handling any new work there should only be one event
in flight of being handled at a time.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/rtas.h |    2 ++
 arch/powerpc/kernel/rtasd.c     |   35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

Comments

Paul Mackerras April 4, 2013, 3:34 a.m. UTC | #1 
On Mon, Mar 25, 2013 at 01:52:32PM -0500, Nathan Fontenot wrote:
> From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
> 
> A PRRN event is signaled via the RTAS event-scan mechanism, which
> returns a Hot Plug Event message "fixed part" indicating "Platform
> Resource Reassignment". In response to the Hot Plug Event message,
> we must call ibm,update-nodes to determine which resources were
> reassigned and then ibm,update-properties to obtain the new affinity
> information about those resources.
> 
> The PRRN event-scan RTAS message contains only the "fixed part" with
> the "Type" field set to the value 160 and no Extended Event Log. The
> four-byte Extended Event Log Length field is repurposed (since no
> Extended Event Log message is included) to pass the "scope" parameter
> that causes the ibm,update-nodes to return the nodes affected by the
> specific resource reassignment.
> 
> This patch adds a handler in rtasd for PRRN RTAS events. The function
> pseries_devicetree_update() (from mobility.c) is used to make the
> ibm,update-nodes/ibm,update-properties RTAS calls. Updating the NUMA maps
> (handled by a subsequent patch) will require significant processing,
> so pseries_devicetree_update() is called from an asynchronous workqueue
> to allow rtasd to continue processing events. Since we flush all work
> on the queue before handling any new work there should only be one event
> in flight of being handled at a time.
            ^^ "of" is superfluous

In the worst case where PRRN events come close together in time, the
flush_work will block for however long it takes to do this
"significant processing", meaning that we're no better off using a
workqueue.  Do we have any reason to think that these PRRN events will
normally be widely spaced in time?  If so you should mention it in the
patch description.

Also, rtasd isn't actually a task, it's just a function that gets run
via schedule_delayed_work_on() and re-schedules itself each time it
runs.  Is there any deadlock possibility in calling flush_work from a
work function?

Paul.
Benjamin Herrenschmidt April 4, 2013, 7:16 a.m. UTC | #2 
On Thu, 2013-04-04 at 14:34 +1100, Paul Mackerras wrote:
> Also, rtasd isn't actually a task, it's just a function that gets run
> via schedule_delayed_work_on() and re-schedules itself each time it
> runs.  Is there any deadlock possibility in calling flush_work from a
> work function?

There used to be, but I'm not familiar with the "new" implementation of
the work queue stuff.

Cheers,
Ben.
Nathan Fontenot April 5, 2013, 3:43 p.m. UTC | #3 
On 04/03/2013 10:34 PM, Paul Mackerras wrote:
> On Mon, Mar 25, 2013 at 01:52:32PM -0500, Nathan Fontenot wrote:
>> From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>>
>> A PRRN event is signaled via the RTAS event-scan mechanism, which
>> returns a Hot Plug Event message "fixed part" indicating "Platform
>> Resource Reassignment". In response to the Hot Plug Event message,
>> we must call ibm,update-nodes to determine which resources were
>> reassigned and then ibm,update-properties to obtain the new affinity
>> information about those resources.
>>
>> The PRRN event-scan RTAS message contains only the "fixed part" with
>> the "Type" field set to the value 160 and no Extended Event Log. The
>> four-byte Extended Event Log Length field is repurposed (since no
>> Extended Event Log message is included) to pass the "scope" parameter
>> that causes the ibm,update-nodes to return the nodes affected by the
>> specific resource reassignment.
>>
>> This patch adds a handler in rtasd for PRRN RTAS events. The function
>> pseries_devicetree_update() (from mobility.c) is used to make the
>> ibm,update-nodes/ibm,update-properties RTAS calls. Updating the NUMA maps
>> (handled by a subsequent patch) will require significant processing,
>> so pseries_devicetree_update() is called from an asynchronous workqueue
>> to allow rtasd to continue processing events. Since we flush all work
>> on the queue before handling any new work there should only be one event
>> in flight of being handled at a time.
>             ^^ "of" is superfluous

will remove it.

> 
> In the worst case where PRRN events come close together in time, the
> flush_work will block for however long it takes to do this
> "significant processing", meaning that we're no better off using a
> workqueue.  Do we have any reason to think that these PRRN events will
> normally be widely spaced in time?  If so you should mention it in the
> patch description.

Yes. PRRN events can only be triggered from the HMC by an IBM tech who has
to actualy log into a customer system and initiate the PRRN event. There
is no method for a user to initiate a PRRN event. Given this is is safe
to assume that these events will be widely spaced in time.

> 
> Also, rtasd isn't actually a task, it's just a function that gets run
> via schedule_delayed_work_on() and re-schedules itself each time it
> runs.  Is there any deadlock possibility in calling flush_work from a
> work function?

I don't know of any but I will investigate.

Thanks for the feedback.
-Nathan
Michael Ellerman April 10, 2013, 8:30 a.m. UTC | #4 
On Mon, Mar 25, 2013 at 01:52:32PM -0500, Nathan Fontenot wrote:
> From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
> 
> A PRRN event is signaled via the RTAS event-scan mechanism, which
> returns a Hot Plug Event message "fixed part" indicating "Platform
> Resource Reassignment". In response to the Hot Plug Event message,
> we must call ibm,update-nodes to determine which resources were
> reassigned and then ibm,update-properties to obtain the new affinity
> information about those resources.
..

> Index: powerpc/arch/powerpc/kernel/rtasd.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/kernel/rtasd.c	2013-03-20 08:24:14.000000000 -0500
> +++ powerpc/arch/powerpc/kernel/rtasd.c	2013-03-20 08:52:08.000000000 -0500
> @@ -87,6 +87,8 @@
>  			return "Resource Deallocation Event";
>  		case RTAS_TYPE_DUMP:
>  			return "Dump Notification Event";
> +		case RTAS_TYPE_PRRN:
> +			return "Platform Resource Reassignment Event";
>  	}
>  
>  	return rtas_type[0];
> @@ -265,7 +267,38 @@
>  		spin_unlock_irqrestore(&rtasd_log_lock, s);
>  		return;
>  	}
> +}
> +
> +static s32 update_scope;
> +
> +static void prrn_work_fn(struct work_struct *work)
> +{
> +	/*
> +	 * For PRRN, we must pass the negative of the scope value in
> +	 * the RTAS event.
> +	 */
> +	pseries_devicetree_update(-update_scope);
> +}
> +static DECLARE_WORK(prrn_work, prrn_work_fn);

This breaks the 32-bit build (ppc6xx_defconfig):

arch/powerpc/kernel/rtasd.c:280: undefined reference to `pseries_devicetree_update'

cheers
Nathan Fontenot April 15, 2013, 8:12 p.m. UTC | #5 
On 04/10/2013 03:30 AM, Michael Ellerman wrote:
> On Mon, Mar 25, 2013 at 01:52:32PM -0500, Nathan Fontenot wrote:
>> From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>>
>> A PRRN event is signaled via the RTAS event-scan mechanism, which
>> returns a Hot Plug Event message "fixed part" indicating "Platform
>> Resource Reassignment". In response to the Hot Plug Event message,
>> we must call ibm,update-nodes to determine which resources were
>> reassigned and then ibm,update-properties to obtain the new affinity
>> information about those resources.
> ..
> 
>> Index: powerpc/arch/powerpc/kernel/rtasd.c
>> ===================================================================
>> --- powerpc.orig/arch/powerpc/kernel/rtasd.c	2013-03-20 08:24:14.000000000 -0500
>> +++ powerpc/arch/powerpc/kernel/rtasd.c	2013-03-20 08:52:08.000000000 -0500
>> @@ -87,6 +87,8 @@
>>  			return "Resource Deallocation Event";
>>  		case RTAS_TYPE_DUMP:
>>  			return "Dump Notification Event";
>> +		case RTAS_TYPE_PRRN:
>> +			return "Platform Resource Reassignment Event";
>>  	}
>>  
>>  	return rtas_type[0];
>> @@ -265,7 +267,38 @@
>>  		spin_unlock_irqrestore(&rtasd_log_lock, s);
>>  		return;
>>  	}
>> +}
>> +
>> +static s32 update_scope;
>> +
>> +static void prrn_work_fn(struct work_struct *work)
>> +{
>> +	/*
>> +	 * For PRRN, we must pass the negative of the scope value in
>> +	 * the RTAS event.
>> +	 */
>> +	pseries_devicetree_update(-update_scope);
>> +}
>> +static DECLARE_WORK(prrn_work, prrn_work_fn);
> 
> This breaks the 32-bit build (ppc6xx_defconfig):
> 
> arch/powerpc/kernel/rtasd.c:280: undefined reference to `pseries_devicetree_update'
> 

I'm not seeing this error. rtasd.c compilkes fine, but I am hitting another
error later in the build that keeps it from finishing.

arch/powerpc/platforms/52xx/mpc52xx_pic.c: In function ‘mpc52xx_irqhost_map’:
arch/powerpc/platforms/52xx/mpc52xx_pic.c:343: error: ‘irqchip’ may be used uninitialized in this function


-Nathan
diff mbox

Patch

Index: powerpc/arch/powerpc/include/asm/rtas.h
===================================================================
--- powerpc.orig/arch/powerpc/include/asm/rtas.h	2013-03-20 08:51:59.000000000 -0500
+++ powerpc/arch/powerpc/include/asm/rtas.h	2013-03-20 08:52:08.000000000 -0500
@@ -143,6 +143,8 @@ 
 #define RTAS_TYPE_PMGM_TIME_ALARM	0x6f
 #define RTAS_TYPE_PMGM_CONFIG_CHANGE	0x70
 #define RTAS_TYPE_PMGM_SERVICE_PROC	0x71
+/* Platform Resource Reassignment Notification */
+#define RTAS_TYPE_PRRN			0xA0
 
 /* RTAS check-exception vector offset */
 #define RTAS_VECTOR_EXTERNAL_INTERRUPT	0x500
Index: powerpc/arch/powerpc/kernel/rtasd.c
===================================================================
--- powerpc.orig/arch/powerpc/kernel/rtasd.c	2013-03-20 08:24:14.000000000 -0500
+++ powerpc/arch/powerpc/kernel/rtasd.c	2013-03-20 08:52:08.000000000 -0500
@@ -87,6 +87,8 @@ 
 			return "Resource Deallocation Event";
 		case RTAS_TYPE_DUMP:
 			return "Dump Notification Event";
+		case RTAS_TYPE_PRRN:
+			return "Platform Resource Reassignment Event";
 	}
 
 	return rtas_type[0];
@@ -265,7 +267,38 @@ 
 		spin_unlock_irqrestore(&rtasd_log_lock, s);
 		return;
 	}
+}
+
+static s32 update_scope;
+
+static void prrn_work_fn(struct work_struct *work)
+{
+	/*
+	 * For PRRN, we must pass the negative of the scope value in
+	 * the RTAS event.
+	 */
+	pseries_devicetree_update(-update_scope);
+}
+static DECLARE_WORK(prrn_work, prrn_work_fn);
+
+void prrn_schedule_update(u32 scope)
+{
+	flush_work(&prrn_work);
+	update_scope = scope;
+	schedule_work(&prrn_work);
+}
+
+static void pseries_handle_event(const struct rtas_error_log *log)
+{
+	pSeries_log_error((char *)log, ERR_TYPE_RTAS_LOG, 0);
+
+	if (log->type == RTAS_TYPE_PRRN)
+		/* For PRRN Events the extended log length is used to denote
+		 * the scope for calling rtas update-nodes.
+		 */
+		prrn_schedule_update(log->extended_log_length);
 
+	return;
 }
 
 static int rtas_log_open(struct inode * inode, struct file * file)
@@ -389,7 +422,7 @@ 
 		}
 
 		if (error == 0)
-			pSeries_log_error(logdata, ERR_TYPE_RTAS_LOG, 0);
+			pseries_handle_event((struct rtas_error_log *)logdata);
 
 	} while(error == 0);
 }