Skip to contentSkip to conversation

Add notification for active Cell SPU tasks

PATCH In review Maynard Johnson 5 participants 10 messages Dec 1, 2006 – Dec 12, 2006

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add notification for active Cell SPU tasks

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

From: Maynard Johnson <[email protected]>

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.

arch/powerpc/platforms/cell/spufs/sched.c +23 -1
Index: linux-2.6.19-rc6-arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c
===================================================================
@@ -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)
#1 lore raw

arch/powerpc/platforms/cell/spufs/sched.c line 99

Quoted 13 lines · Maynard Johnson
arch/powerpc/platforms/cell/spufs/sched.c +13 -0
+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 <><
#2 lore raw
Quoted 11 lines · Arnd Bergmann
Quoted 9 lines · Maynard Johnson
arch/powerpc/platforms/cell/spufs/sched.c +9 -0
+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,
Quoted 8 lines

arch/powerpc/platforms/cell/spufs/sched.c line 99

Quoted 4 lines
arch/powerpc/platforms/cell/spufs/sched.c +4 -0
+                                                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.

Quoted 5 lines

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 <><
#3 lore raw
In reply to Maynard Johnson

[email protected] wrote on 01/12/2006 06:01:15 PM:

Quoted 2 lines

Subject: Enable SPU switch notification to detect currently active SPU

tasks.

Quoted 4 lines

From: Maynard Johnson <[email protected]>

This patch adds to the capability of spu_switch_event_register to notify

the

Quoted 45 lines

caller of currently active SPU tasks. It also exports spu_switch_event_register
and spu_switch_event_unregister.

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

arch/powerpc/platforms/cell/spufs/sched.c +21 -1
@@ -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;
 }

arch/powerpc/platforms/cell/spufs/sched.c line 120

Quoted 10 lines
arch/powerpc/platforms/cell/spufs/sched.c +2 -0
+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

#4 lore raw

[email protected] wrote on 12/04/2006 10:26:57:

Quoted 5 lines

[email protected] wrote on 01/12/2006 06:01:15 PM:

Quoted 2 lines

Subject: Enable SPU switch notification to detect currently activeSPU

tasks.

Quoted 4 lines
Quoted 4 lines

From: Maynard Johnson <[email protected]>

This patch adds to the capability of spu_switch_event_register to

notify the

Quoted 47 lines
Quoted 47 lines

caller of currently active SPU tasks. It also exports spu_switch_event_register
and spu_switch_event_unregister.

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

arch/powerpc/platforms/cell/spufs/sched.c +22 -1
@@ -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);
 
Quoted 8 lines

arch/powerpc/platforms/cell/spufs/sched.c

Quoted 8 lines
arch/powerpc/platforms/cell/spufs/sched.c +1 -0
 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)

Quoted 5 lines

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

Quoted 7 lines

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

Quoted 10 lines

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

#5 lore raw
Quoted 68 lines · Luke Browning

[email protected] wrote on 12/04/2006 10:26:57:

Quoted 5 lines

[email protected] wrote on 01/12/2006 06:01:15 PM:

Quoted 2 lines

Subject: Enable SPU switch notification to detect currently

activeSPU tasks.

Quoted 4 lines
Quoted 4 lines

From: Maynard Johnson <[email protected]>

This patch adds to the capability of spu_switch_event_register to

notify the

Quoted 47 lines
Quoted 47 lines

caller of currently active SPU tasks. It also exports spu_switch_event_register
and spu_switch_event_unregister.

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

arch/powerpc/platforms/cell/spufs/sched.c +22 -1
@@ -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);
 
Quoted 3 lines

arch/powerpc/platforms/cell/spufs/sched.c

Quoted 3 lines
arch/powerpc/platforms/cell/spufs/sched.c +0 -0
 int spu_switch_event_unregister(struct notifier_block * n)
 {
    return 

blocking_notifier_chain_unregister(&spu_switch_notifier, n);

Quoted 4 lines
Quoted 4 lines
arch/powerpc/platforms/cell/spufs/sched.c +1 -0
 }
+EXPORT_SYMBOL_GPL(spu_switch_event_unregister);
 
 
Quoted 7 lines
Quoted 1 line

Review line 120

Quoted 1 line
(quoted diff) +0 -0
 static inline void bind_context(struct spu *spu, struct 

spu_context *ctx)

Quoted 2 lines

Is this really the right strategy? First, it serializes all spu

context

Quoted 1 line

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.

Quoted 5 lines
Quoted 2 lines

every context
switch. Can't oprofile internally derive the list of active spus

from the

Quoted 1 line

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?

Quoted 9 lines
Quoted 2 lines

Also, the notify_spus_active() callout is dependent on the return

code of

Quoted 3 lines

spu_switch_notify(). Should notification be hierarchical? If I only register
for the second one, should my notification be dependent on the

return code

Quoted 1 line

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.

Quoted 7 lines
Quoted 6 lines

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.

Quoted 8 lines
Quoted 3 lines

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

Quoted 10 lines

Luke

------------------------------------------------------------------------

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev

#6 lore raw
Quoted 8 lines · Maynard Johnson

text(struct spu *spu, struct=20

Quoted 6 lines

spu_context *ctx)

Quoted 2 lines

Is this really the right strategy? =A0First, it serializes all spu=20

context

Quoted 1 line

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

Quoted 7 lines

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

Quoted 2 lines
Quoted 2 lines

every context
switch. =A0Can't oprofile internally derive the list of active spus=20

Quoted 8 lines
Quoted 3 lines

from the =A0

Quoted 1 line

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.

Quoted 10 lines
Quoted 9 lines
Quoted 2 lines

Also, the notify_spus_active() callout is dependent on the return=20

code of

Quoted 3 lines

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

Quoted 1 line

of some non-related subsystem's handler.=20

I'm not exactly sure what you're saying here. =A0Are you suggesting that =

a=20

Quoted 6 lines

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

Quoted 2 lines

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 <><
#7 lore raw
Quoted 75 lines · Arnd Bergmann
Quoted 23 lines · Maynard Johnson

text(struct spu *spu, struct

Quoted 12 lines

spu_context *ctx)

Quoted 3 lines

Is this really the right strategy? First, it serializes all spu

context

Quoted 3 lines

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.

Quoted 4 lines
Quoted 4 lines

every context
switch. Can't oprofile internally derive the list of active spus

Quoted 9 lines
Quoted 6 lines

from the

Quoted 3 lines

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.

Quoted 17 lines
Quoted 17 lines
Quoted 3 lines

Also, the notify_spus_active() callout is dependent on the return

code of

Quoted 5 lines

spu_switch_notify(). Should notification be hierarchical? If I only register
for the second one, should my notification be dependent on the

return code

Quoted 3 lines

of some non-related subsystem's handler.

Quoted 10 lines
Quoted 4 lines

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

Quoted 13 lines
	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
[email protected]
https://lists.sourceforge.net/lists/listinfo/oprofile-list

#8 lore raw

[email protected] wrote on 08/12/2006 01:04:30 PM:

Quoted 8 lines
Quoted 6 lines · Arnd Bergmann

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.

Quoted 8 lines
Quoted 8 lines

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

Quoted 1 line
Quoted 1 line

just point to the currently active object, this should be really

cheap.

Quoted 4 lines
Quoted 4 lines

- 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.

Quoted 1 line
Quoted 1 line

Agreed.

<snip>

Quoted 1 line
Quoted 1 line
Quoted 1 line

I'm not exactly sure what you're saying here. Are you suggesting that

a

Quoted 6 lines
Quoted 5 lines
Quoted 5 lines

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

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

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

Quoted 1 line
Quoted 1 line

not call blocking_notifier_call_chain(), since it will notify other

users

Quoted 9 lines
Quoted 4 lines

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

#9 lore raw
Quoted 56 lines · Luke Browning

[email protected] wrote on 08/12/2006 01:04:30 PM:

Quoted 8 lines
Quoted 6 lines · Arnd Bergmann

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.

Quoted 8 lines
Quoted 8 lines

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

Quoted 1 line
Quoted 1 line

just point to the currently active object, this should be really

cheap.

Quoted 4 lines
Quoted 4 lines

- 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.

Quoted 1 line
Quoted 1 line

Agreed.

<snip>

Quoted 1 line
Quoted 1 line
Quoted 1 line

I'm not exactly sure what you're saying here. Are you suggesting

that a

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

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

Quoted 1 line
Quoted 1 line

not call blocking_notifier_call_chain(), since it will notify other

users

Quoted 7 lines
Quoted 4 lines

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.

Quoted 7 lines
Quoted 2 lines

-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 *.

Quoted 4 lines

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.

Quoted 3 lines

Luke