[PATCH]Add notification for active Cell SPU tasks

[PATCH]Add notification for active Cell SPU tasks

[Maynard Johnson]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: spu-notifier.patch --]
[-- Type: text/x-diff, Size: 1828 bytes --]

Subject: Enable SPU switch notification to detect currently active SPU tasks.

From: Maynard Johnson <maynardj@us.ibm.com>

This patch adds to the capability of spu_switch_event_register to notify the
caller of currently active SPU tasks.  It also exports spu_switch_event_register
and spu_switch_event_unregister.

Signed-off-by: Maynard Johnson <mpjohn@us.ibm.com>


Index: linux-2.6.19-rc6-arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c
===================================================================
--- linux-2.6.19-rc6-arnd1+patches.orig/arch/powerpc/platforms/cell/spufs/sched.c	2006-11-24 11:34:44.884455680 -0600
+++ linux-2.6.19-rc6-arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c	2006-12-01 13:57:21.864583264 -0600
@@ -84,15 +84,37 @@
 			    ctx ? ctx->object_id : 0, spu);
 }
 
+static void notify_spus_active(void)
+{
+	int node;
+	for (node = 0; node < MAX_NUMNODES; node++) {
+		struct spu *spu;
+		mutex_lock(&spu_prio->active_mutex[node]);
+		list_for_each_entry(spu, &spu_prio->active_list[node], list) {
+	 			 struct spu_context *ctx = spu->ctx;
+	 			 blocking_notifier_call_chain(&spu_switch_notifier,
+	 		 			 ctx ? ctx->object_id : 0, spu);
+		}
+		mutex_unlock(&spu_prio->active_mutex[node]);
+	 }
+
+}
+
 int spu_switch_event_register(struct notifier_block * n)
 {
-	return blocking_notifier_chain_register(&spu_switch_notifier, n);
+	int ret;
+	ret = blocking_notifier_chain_register(&spu_switch_notifier, n);
+	if (!ret)
+		notify_spus_active();
+	return ret;
 }
+EXPORT_SYMBOL_GPL(spu_switch_event_register);
 
 int spu_switch_event_unregister(struct notifier_block * n)
 {
 	return blocking_notifier_chain_unregister(&spu_switch_notifier, n);
 }
+EXPORT_SYMBOL_GPL(spu_switch_event_unregister);
 
 
 static inline void bind_context(struct spu *spu, struct spu_context *ctx)

Re: [Cbe-oss-dev] [PATCH]Add notification for active Cell SPU tasks

[Arnd Bergmann]

On Friday 01 December 2006 21:01, Maynard Johnson wrote:
> +static void notify_spus_active(void)
> +{
> +       int node;
> +       for (node = 0; node < MAX_NUMNODES; node++) {
> +               struct spu *spu;
> +               mutex_lock(&spu_prio->active_mutex[node]);
> +               list_for_each_entry(spu, &spu_prio->active_list[node], list) {
> +                                struct spu_context *ctx = spu->ctx;
> +                                blocking_notifier_call_chain(&spu_switch_notifier,
> +                                                ctx ? ctx->object_id : 0, spu);
> +               }
> +               mutex_unlock(&spu_prio->active_mutex[node]);
> +        }

I wonder if this is enough for oprofile. Don't you need to access user
space data of the task running on the SPU? I always assumed you want
to do it via get_user or copy_from_user, which obviously doesn't work
here, when you're running in the oprofile task. Are you using something
like access_process_vm now?

	Arnd <><

Re: [Cbe-oss-dev] [PATCH]Add notification for active Cell SPU tasks

[Maynard Johnson]

Arnd Bergmann wrote:
> On Friday 01 December 2006 21:01, Maynard Johnson wrote:
> 
>>+static void notify_spus_active(void)
>>+{
>>+       int node;
>>+       for (node = 0; node < MAX_NUMNODES; node++) {
>>+               struct spu *spu;
>>+               mutex_lock(&spu_prio->active_mutex[node]);
>>+               list_for_each_entry(spu, &spu_prio->active_list[node], list) {
>>+                                struct spu_context *ctx = spu->ctx;
>>+                                blocking_notifier_call_chain(&spu_switch_notifier,
>>+                                                ctx ? ctx->object_id : 0, spu);
>>+               }
>>+               mutex_unlock(&spu_prio->active_mutex[node]);
>>+        }
> 
> 
> I wonder if this is enough for oprofile. Don't you need to access user
> space data of the task running on the SPU? I always assumed you want
No, I don't need anything from the user app besides access to the 
executable binary, which I get with copy_from_user, specifying the 
'objectid' as from address.
> to do it via get_user or copy_from_user, which obviously doesn't work
> here, when you're running in the oprofile task. Are you using something
> like access_process_vm now?
> 
> 	Arnd <><

Re: [PATCH]Add notification for active Cell SPU tasks

[Luke Browning]

[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]







linuxppc-dev-bounces+lukebrowning=us.ibm.com@ozlabs.org wrote on 01/12/2006
06:01:15 PM:

>
> Subject: Enable SPU switch notification to detect currently active SPU
tasks.
>
> From: Maynard Johnson <maynardj@us.ibm.com>
>
> This patch adds to the capability of spu_switch_event_register to notify
the
> caller of currently active SPU tasks.  It also exports
> spu_switch_event_register
> and spu_switch_event_unregister.
>
> Signed-off-by: Maynard Johnson <mpjohn@us.ibm.com>
>
>
> Index: linux-2.6.19-rc6-
> arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c
> ===================================================================
> --- linux-2.6.19-rc6-arnd1+patches.
> orig/arch/powerpc/platforms/cell/spufs/sched.c   2006-11-24 11:34:
> 44.884455680 -0600
> +++ linux-2.6.19-rc6-
> arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c   2006-12-01
> 13:57:21.864583264 -0600
> @@ -84,15 +84,37 @@
>               ctx ? ctx->object_id : 0, spu);
>  }
>
> +static void notify_spus_active(void)
> +{
> +   int node;
> +   for (node = 0; node < MAX_NUMNODES; node++) {
> +      struct spu *spu;
> +      mutex_lock(&spu_prio->active_mutex[node]);
> +      list_for_each_entry(spu, &spu_prio->active_list[node], list) {
> +              struct spu_context *ctx = spu->ctx;
> +              blocking_notifier_call_chain(&spu_switch_notifier,
> +                     ctx ? ctx->object_id : 0, spu);
> +      }
> +      mutex_unlock(&spu_prio->active_mutex[node]);
> +    }
> +
> +}
> +
>  int spu_switch_event_register(struct notifier_block * n)
>  {
> -   return blocking_notifier_chain_register(&spu_switch_notifier, n);
> +   int ret;
> +   ret = blocking_notifier_chain_register(&spu_switch_notifier, n);
> +   if (!ret)
> +      notify_spus_active();
> +   return ret;
>  }
> +EXPORT_SYMBOL_GPL(spu_switch_event_register);
>
>  int spu_switch_event_unregister(struct notifier_block * n)
>  {
>     return blocking_notifier_chain_unregister(&spu_switch_notifier, n);
>  }
> +EXPORT_SYMBOL_GPL(spu_switch_event_unregister);
>
>
>  static inline void bind_context(struct spu *spu, struct spu_context
*ctx)

Is this really the right strategy?  First, it serializes all spu context
switching at the node level.  Second, it performs 17 callouts for every
context
switch.  Can't oprofile internally derive the list of active spus from the
context switch callout.

Also, the notify_spus_active() callout is dependent on the return code of
spu_switch_notify().  Should notification be hierarchical?  If I only
register
for the second one, should my notification be dependent on the return code
of some non-related subsystem's handler.

Does blocking_callchain_notifier internally check for the presence of
registered
handlers before it takes locks ...?  We should ensure that there is minimal
overhead
when there are no registered handlers.

Regards,
Luke

[-- Attachment #2: Type: text/html, Size: 4019 bytes --]

Re: [PATCH]Add notification for active Cell SPU tasks

[Luke Browning]

[-- Attachment #1: Type: text/plain, Size: 3345 bytes --]





linuxppc-dev-bounces+lukebrowning=us.ibm.com@ozlabs.org wrote on 12/04/2006
10:26:57:

> linuxppc-dev-bounces+lukebrowning=us.ibm.com@ozlabs.org wrote on
> 01/12/2006 06:01:15 PM:
>
> >
> > Subject: Enable SPU switch notification to detect currently activeSPU
tasks.
> >
> > From: Maynard Johnson <maynardj@us.ibm.com>
> >
> > This patch adds to the capability of spu_switch_event_register to
notify the
> > caller of currently active SPU tasks.  It also exports
> > spu_switch_event_register
> > and spu_switch_event_unregister.
> >
> > Signed-off-by: Maynard Johnson <mpjohn@us.ibm.com>
> >
> >
> > Index: linux-2.6.19-rc6-
> > arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c
> > ===================================================================
> > --- linux-2.6.19-rc6-arnd1+patches.
> > orig/arch/powerpc/platforms/cell/spufs/sched.c   2006-11-24 11:34:
> > 44.884455680 -0600
> > +++ linux-2.6.19-rc6-
> > arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c   2006-12-01
> > 13:57:21.864583264 -0600
> > @@ -84,15 +84,37 @@
> >               ctx ? ctx->object_id : 0, spu);
> >  }
> >
> > +static void notify_spus_active(void)
> > +{
> > +   int node;
> > +   for (node = 0; node < MAX_NUMNODES; node++) {
> > +      struct spu *spu;
> > +      mutex_lock(&spu_prio->active_mutex[node]);
> > +      list_for_each_entry(spu, &spu_prio->active_list[node], list) {
> > +              struct spu_context *ctx = spu->ctx;
> > +              blocking_notifier_call_chain(&spu_switch_notifier,
> > +                     ctx ? ctx->object_id : 0, spu);
> > +      }
> > +      mutex_unlock(&spu_prio->active_mutex[node]);
> > +    }
> > +
> > +}
> > +
> >  int spu_switch_event_register(struct notifier_block * n)
> >  {
> > -   return blocking_notifier_chain_register(&spu_switch_notifier, n);
> > +   int ret;
> > +   ret = blocking_notifier_chain_register(&spu_switch_notifier, n);
> > +   if (!ret)
> > +      notify_spus_active();
> > +   return ret;
> >  }
> > +EXPORT_SYMBOL_GPL(spu_switch_event_register);
> >
> >  int spu_switch_event_unregister(struct notifier_block * n)
> >  {
> >     return blocking_notifier_chain_unregister(&spu_switch_notifier, n);
> >  }
> > +EXPORT_SYMBOL_GPL(spu_switch_event_unregister);
> >
> >
> >  static inline void bind_context(struct spu *spu, struct spu_context
*ctx)
>
> Is this really the right strategy?  First, it serializes all spu context
> switching at the node level.  Second, it performs 17 callouts for
> every context
> switch.  Can't oprofile internally derive the list of active spus from
the
> context switch callout.
>
> Also, the notify_spus_active() callout is dependent on the return code of

> spu_switch_notify().  Should notification be hierarchical?  If I
> only register
> for the second one, should my notification be dependent on the return
code
> of some non-related subsystem's handler.
>
> Does blocking_callchain_notifier internally check for the presence
> of registered
> handlers before it takes locks ...?  We should ensure that there is
> minimal overhead
> when there are no registered handlers.
>
> Regards,
> Luke___________________

Any comments to my questions above.  Seems like oprofile / pdt could derive
the
list of active spus from a single context switch callout.  This patch will
have
a large impact on the performance of the system.

Luke

[-- Attachment #2: Type: text/html, Size: 4748 bytes --]

Re: [PATCH]Add notification for active Cell SPU tasks

[Maynard Johnson]

Luke Browning wrote:

> linuxppc-dev-bounces+lukebrowning=us.ibm.com@ozlabs.org wrote on 
> 12/04/2006 10:26:57:
>
> > linuxppc-dev-bounces+lukebrowning=us.ibm.com@ozlabs.org wrote on
> > 01/12/2006 06:01:15 PM:
> >
> > >
> > > Subject: Enable SPU switch notification to detect currently 
> activeSPU tasks.
> > >
> > > From: Maynard Johnson <maynardj@us.ibm.com>
> > >
> > > This patch adds to the capability of spu_switch_event_register to 
> notify the
> > > caller of currently active SPU tasks.  It also exports
> > > spu_switch_event_register
> > > and spu_switch_event_unregister.
> > >
> > > Signed-off-by: Maynard Johnson <mpjohn@us.ibm.com>
> > >
> > >
> > > Index: linux-2.6.19-rc6-
> > > arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c
> > > ===================================================================
> > > --- linux-2.6.19-rc6-arnd1+patches.
> > > orig/arch/powerpc/platforms/cell/spufs/sched.c   2006-11-24 11:34:
> > > 44.884455680 -0600
> > > +++ linux-2.6.19-rc6-
> > > arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c   2006-12-01
> > > 13:57:21.864583264 -0600
> > > @@ -84,15 +84,37 @@
> > >               ctx ? ctx->object_id : 0, spu);
> > >  }
> > >  
> > > +static void notify_spus_active(void)
> > > +{
> > > +   int node;
> > > +   for (node = 0; node < MAX_NUMNODES; node++) {
> > > +      struct spu *spu;
> > > +      mutex_lock(&spu_prio->active_mutex[node]);
> > > +      list_for_each_entry(spu, &spu_prio->active_list[node], list) {
> > > +              struct spu_context *ctx = spu->ctx;
> > > +              blocking_notifier_call_chain(&spu_switch_notifier,
> > > +                     ctx ? ctx->object_id : 0, spu);
> > > +      }
> > > +      mutex_unlock(&spu_prio->active_mutex[node]);
> > > +    }
> > > +
> > > +}
> > > +
> > >  int spu_switch_event_register(struct notifier_block * n)
> > >  {
> > > -   return blocking_notifier_chain_register(&spu_switch_notifier, n);
> > > +   int ret;
> > > +   ret = blocking_notifier_chain_register(&spu_switch_notifier, n);
> > > +   if (!ret)
> > > +      notify_spus_active();
> > > +   return ret;
> > >  }
> > > +EXPORT_SYMBOL_GPL(spu_switch_event_register);
> > >  
> > >  int spu_switch_event_unregister(struct notifier_block * n)
> > >  {
> > >     return 
> blocking_notifier_chain_unregister(&spu_switch_notifier, n);
> > >  }
> > > +EXPORT_SYMBOL_GPL(spu_switch_event_unregister);
> > >  
> > >  
> > >  static inline void bind_context(struct spu *spu, struct 
> spu_context *ctx)
> >
> > Is this really the right strategy?  First, it serializes all spu 
> context
> > switching at the node level.  Second, it performs 17 callouts for
>
I could be wrong, but I think if we moved the mutex_lock to be inside of 
the list_for_each_entry loop, we could have a race condition.  For 
example, we obtain the next spu item from the spu_prio->active_mutex 
list, then wait on the mutex which is being held for the purpose of 
removing the very spu context we just obtained.

> > every context
> > switch.  Can't oprofile internally derive the list of active spus 
> from the  
> > context switch callout. 
>
Arnd would certainly know the answer to this off the top of his head, 
but when I initially discussed the idea for this patch with him 
(probably a couple months ago or so), he didn't suggest a better 
alternative.  Perhaps there is a way to do this with current SPUFS 
code.  Arnd, any comments on this?

> >
> > Also, the notify_spus_active() callout is dependent on the return 
> code of
> > spu_switch_notify().  Should notification be hierarchical?  If I
> > only register
> > for the second one, should my notification be dependent on the 
> return code
> > of some non-related subsystem's handler. 
>
I'm not exactly sure what you're saying here.  Are you suggesting that a 
user may only be interested in acitve SPU notification and, therefore, 
shouldn't have to be depenent on the "standard" notification 
registration succeeding?  There may be a case for adding a new 
registration function, I suppose; although, I'm not aware of any other 
users of the SPUFS notification mechanism besides OProfile and PDT, and 
we need notification of both active and future SPU tasks.  But I would 
not object to a new function.

> >
> > Does blocking_callchain_notifier internally check for the presence
> > of registered
> > handlers before it takes locks ...?  We should ensure that there is
> > minimal overhead
> > when there are no registered handlers.
>
I won't pretend to be expert enough to critique the performance of that 
code.

> >
> > Regards,
> > Luke___________________
>
> Any comments to my questions above.  Seems like oprofile / pdt could 
> derive the
> list of active spus from a single context switch callout.  This patch 
> will have
> a large impact on the performance of the system.
>
For OProfile, the registration is only done at the time when a user 
starts the profiler to collect performance data, typically focusing on a 
single application, so I don't see this as an impact on normal 
production operations.  Since you must have root authority to run 
OProfile, it cannot be invoked by just any user for nefarious purposes.

-Maynard

>
> Luke
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@ozlabs.org
>https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

Re: [Cbe-oss-dev] [PATCH]Add notification for active Cell SPU tasks

[Arnd Bergmann]

On Wednesday 06 December 2006 23:04, Maynard Johnson wrote:
> text(struct spu *spu, struct=20
> > spu_context *ctx)
> > >
> > > Is this really the right strategy? =A0First, it serializes all spu=20
> > context
> > > switching at the node level. =A0Second, it performs 17 callouts for
> >
> I could be wrong, but I think if we moved the mutex_lock to be inside of=
=20
> the list_for_each_entry loop, we could have a race condition. =A0For=20
> example, we obtain the next spu item from the spu_prio->active_mutex=20
> list, then wait on the mutex which is being held for the purpose of=20
> removing the very spu context we just obtained.
>=20
> > > every context
> > > switch. =A0Can't oprofile internally derive the list of active spus=20
> > from the =A0
> > > context switch callout.=20
> >
> Arnd would certainly know the answer to this off the top of his head,=20
> but when I initially discussed the idea for this patch with him=20
> (probably a couple months ago or so), he didn't suggest a better=20
> alternative. =A0Perhaps there is a way to do this with current SPUFS=20
> code. =A0Arnd, any comments on this?



No code should ever need to look at other SPUs when performing an
operation on a given SPU, so we don't need to hold a global lock
during normal operation.

We have two cases that need to be handled:

=2D on each context unload and load (both for a full switch operation),
  call to the profiling code with a pointer to the current context
  and spu (context is NULL when unloading).

  If the new context is not know yet, scan its overlay table (expensive)
  and store information about it in an oprofile private object. Otherwise
  just point to the currently active object, this should be really cheap.

=2D When enabling oprofile initially, scan all contexts that are currently
  running on one of the SPUs. This is also expensive, but should happen
  before the measurement starts so it does not impact the resulting data.

> > >
> > > Also, the notify_spus_active() callout is dependent on the return=20
> > code of
> > > spu_switch_notify(). =A0Should notification be hierarchical? =A0If I
> > > only register
> > > for the second one, should my notification be dependent on the=20
> > return code
> > > of some non-related subsystem's handler.=20
> >
> I'm not exactly sure what you're saying here. =A0Are you suggesting that =
a=20
> user may only be interested in acitve SPU notification and, therefore,=20
> shouldn't have to be depenent on the "standard" notification=20
> registration succeeding? =A0There may be a case for adding a new=20
> registration function, I suppose; although, I'm not aware of any other=20
> users of the SPUFS notification mechanism besides OProfile and PDT, and=20
> we need notification of both active and future SPU tasks. =A0But I would=
=20
> not object to a new function.
>=20
I think what Luke was trying to get to is that notify_spus_active() should
not call blocking_notifier_call_chain(), since it will notify other users
as well as the newly registered one. Instead, it can simply call the
notifier function directly.

	Arnd <><

Re: [Cbe-oss-dev] [PATCH]Add notification for active Cell SPU tasks

[Maynard Johnson]

Arnd Bergmann wrote:

>On Wednesday 06 December 2006 23:04, Maynard Johnson wrote:
>  
>
>>text(struct spu *spu, struct 
>>    
>>
>>>spu_context *ctx)
>>>      
>>>
>>>>Is this really the right strategy?  First, it serializes all spu 
>>>>        
>>>>
>>>context
>>>      
>>>
>>>>switching at the node level.  Second, it performs 17 callouts for
>>>>        
>>>>
>>I could be wrong, but I think if we moved the mutex_lock to be inside of 
>>the list_for_each_entry loop, we could have a race condition.  For 
>>example, we obtain the next spu item from the spu_prio->active_mutex 
>>list, then wait on the mutex which is being held for the purpose of 
>>removing the very spu context we just obtained.
>>
>>    
>>
>>>>every context
>>>>switch.  Can't oprofile internally derive the list of active spus 
>>>>        
>>>>
>>>from the  
>>>      
>>>
>>>>context switch callout. 
>>>>        
>>>>
>>Arnd would certainly know the answer to this off the top of his head, 
>>but when I initially discussed the idea for this patch with him 
>>(probably a couple months ago or so), he didn't suggest a better 
>>alternative.  Perhaps there is a way to do this with current SPUFS 
>>code.  Arnd, any comments on this?
>>    
>>
>
>
>
>No code should ever need to look at other SPUs when performing an
>operation on a given SPU, so we don't need to hold a global lock
>during normal operation.
>
>We have two cases that need to be handled:
>
>- on each context unload and load (both for a full switch operation),
>  call to the profiling code with a pointer to the current context
>  and spu (context is NULL when unloading).
>
>  If the new context is not know yet, scan its overlay table (expensive)
>  and store information about it in an oprofile private object. Otherwise
>  just point to the currently active object, this should be really cheap.
>
>- When enabling oprofile initially, scan all contexts that are currently
>  running on one of the SPUs. This is also expensive, but should happen
>  before the measurement starts so it does not impact the resulting data.
>
>  
>
>>>>Also, the notify_spus_active() callout is dependent on the return 
>>>>        
>>>>
>>>code of
>>>      
>>>
>>>>spu_switch_notify().  Should notification be hierarchical?  If I
>>>>only register
>>>>for the second one, should my notification be dependent on the 
>>>>        
>>>>
>>>return code
>>>      
>>>
>>>>of some non-related subsystem's handler. 
>>>>        
>>>>
>>I'm not exactly sure what you're saying here.  Are you suggesting that a 
>>user may only be interested in acitve SPU notification and, therefore, 
>>shouldn't have to be depenent on the "standard" notification 
>>registration succeeding?  There may be a case for adding a new 
>>registration function, I suppose; although, I'm not aware of any other 
>>users of the SPUFS notification mechanism besides OProfile and PDT, and 
>>we need notification of both active and future SPU tasks.  But I would 
>>not object to a new function.
>>
>>    
>>
>I think what Luke was trying to get to is that notify_spus_active() should
>not call blocking_notifier_call_chain(), since it will notify other users
>as well as the newly registered one. Instead, it can simply call the
>notifier function directly.
>  
>
Ah, yes.  Thanks to both of you for pointing that out.  I'll fix that 
and re-post.

-Maynard

>	Arnd <><
>
>-------------------------------------------------------------------------
>Take Surveys. Earn Cash. Influence the Future of IT
>Join SourceForge.net's Techsay panel and you'll get the chance to share your
>opinions on IT & business topics through brief surveys - and earn cash
>http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>_______________________________________________
>oprofile-list mailing list
>oprofile-list@lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/oprofile-list
>  
>

Re: [Cbe-oss-dev] [PATCH]Add notification for active Cell SPU tasks

[Luke Browning]

[-- Attachment #1: Type: text/plain, Size: 2375 bytes --]







maynardj@linux.vnet.ibm.com wrote on 08/12/2006 01:04:30 PM:

> Arnd Bergmann wrote:
>
> >On Wednesday 06 December 2006 23:04, Maynard Johnson wrote:
> >
> >No code should ever need to look at other SPUs when performing an
> >operation on a given SPU, so we don't need to hold a global lock
> >during normal operation.
> >
> >We have two cases that need to be handled:
> >
> >- on each context unload and load (both for a full switch operation),
> >  call to the profiling code with a pointer to the current context
> >  and spu (context is NULL when unloading).
> >
> >  If the new context is not know yet, scan its overlay table (expensive)
> >  and store information about it in an oprofile private object.
Otherwise
> >  just point to the currently active object, this should be really
cheap.
> >
> >- When enabling oprofile initially, scan all contexts that are currently
> >  running on one of the SPUs. This is also expensive, but should happen
> >  before the measurement starts so it does not impact the resulting
data.
> >

Agreed.

<snip>

> >>I'm not exactly sure what you're saying here.  Are you suggesting that
a
> >>user may only be interested in acitve SPU notification and, therefore,
> >>shouldn't have to be depenent on the "standard" notification
> >>registration succeeding?  There may be a case for adding a new
> >>registration function, I suppose; although, I'm not aware of any other
> >>users of the SPUFS notification mechanism besides OProfile and PDT, and

> >>we need notification of both active and future SPU tasks.  But I would
> >>not object to a new function.
> >>
> >>
> >>
> >I think what Luke was trying to get to is that notify_spus_active()
should
> >not call blocking_notifier_call_chain(), since it will notify other
users
> >as well as the newly registered one. Instead, it can simply call the
> >notifier function directly.
> >
> >
> Ah, yes.  Thanks to both of you for pointing that out.  I'll fix that
> and re-post.
>
> -Maynard
>

I actually was hoping to take this one step further.  If the interface to
the context switch handler is something like:

switch_handler(int spu_id, from_ctx, to_ctx)

The kernel extension can maintain an internal spu table of its own where it
marks the named spuid as active or not.  You don't need to have a bunch of
individual calls.  Internally, you can keep track of it yourself.

Luke

[-- Attachment #2: Type: text/html, Size: 3154 bytes --]

Re: [Cbe-oss-dev] [PATCH]Add notification for active Cell SPU tasks

[Maynard Johnson]

Luke Browning wrote:
> maynardj@linux.vnet.ibm.com wrote on 08/12/2006 01:04:30 PM:
> 
>  > Arnd Bergmann wrote:
>  >
>  > >On Wednesday 06 December 2006 23:04, Maynard Johnson wrote:
>  > >  
>  > >No code should ever need to look at other SPUs when performing an
>  > >operation on a given SPU, so we don't need to hold a global lock
>  > >during normal operation.
>  > >
>  > >We have two cases that need to be handled:
>  > >
>  > >- on each context unload and load (both for a full switch operation),
>  > >  call to the profiling code with a pointer to the current context
>  > >  and spu (context is NULL when unloading).
>  > >
>  > >  If the new context is not know yet, scan its overlay table (expensive)
>  > >  and store information about it in an oprofile private object. 
> Otherwise
>  > >  just point to the currently active object, this should be really 
> cheap.
>  > >
>  > >- When enabling oprofile initially, scan all contexts that are currently
>  > >  running on one of the SPUs. This is also expensive, but should happen
>  > >  before the measurement starts so it does not impact the resulting 
> data.
>  > >
> 
> Agreed.  
> 
> <snip>
> 
>  > >>I'm not exactly sure what you're saying here.  Are you suggesting 
> that a
>  > >>user may only be interested in acitve SPU notification and, therefore,
>  > >>shouldn't have to be depenent on the "standard" notification
>  > >>registration succeeding?  There may be a case for adding a new
>  > >>registration function, I suppose; although, I'm not aware of any other
>  > >>users of the SPUFS notification mechanism besides OProfile and PDT, 
> and
>  > >>we need notification of both active and future SPU tasks.  But I would
>  > >>not object to a new function.
>  > >>
>  > >>    
>  > >>
>  > >I think what Luke was trying to get to is that notify_spus_active() 
> should
>  > >not call blocking_notifier_call_chain(), since it will notify other 
> users
>  > >as well as the newly registered one. Instead, it can simply call the
>  > >notifier function directly.
>  > >  
>  > >
>  > Ah, yes.  Thanks to both of you for pointing that out.  I'll fix that
>  > and re-post.
>  >
>  > -Maynard
>  >
> 
> I actually was hoping to take this one step further.  If the interface to
> the context switch handler is something like:
> 
> switch_handler(int spu_id, from_ctx, to_ctx)
The function prototype for the switch handler is set in concrete by the 
notification framework.  The parameters are: struct notifier_block *, 
unsigned long, void *.
> 
> The kernel extension can maintain an internal spu table of its own where it
> marks the named spuid as active or not.  You don't need to have a bunch of
> individual calls.  Internally, you can keep track of it yourself.
I think this would be nice to have, and I will look into it as I have 
time.  However, for the existing usage of the SPU switch notification, I 
don't think it's too critical, since most users are not going to be 
trying to do profiling or debugging with multiple SPU apps running 
simultaneously.
> 
> Luke
>