Re: [PATCH] Cell SPU task notification -- updated patch: #1



Attached is an updated patch that addresses Michael Ellerman's comments.

One comment made by Michael has not yet been addressed:
The comment was in regard to the for-loop in spufs/sched.c:notify_spus_active(). He wondered if the scheduler can swap a context from one node to another. If so, there's a small window in this loop (where we switch the lock from one node's active list to the next) where it may be possible we might miss waking up a context and send a spurious wakeup to another.
Arnd . . . can you comment on this question?

Thanks.
-Maynard

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

From: Maynard Johnson <maynardj@xxxxxxxxxx>

This patch adds to the capability of spu_switch_event_register so that the
caller is also notified of currently active SPU tasks. It also exports
spu_switch_event_register and spu_switch_event_unregister.

Signed-off-by: Maynard Johnson <mpjohn@xxxxxxxxxx>


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-12-04 10:56:04.730698720 -0600
+++ linux-2.6.19-rc6-arnd1+patches/arch/powerpc/platforms/cell/spufs/sched.c 2007-01-15 16:22:31.808461448 -0600
@@ -84,15 +84,42 @@
ctx ? ctx->object_id : 0, spu);
}

+static void notify_spus_active(void)
+{
+ int node;
+ /* Wake up the active spu_contexts. When the awakened processes
+ * sees their notify_active flag is set, they will call
+ * spu_notify_already_active().
+ */
+ 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;
+ spu->notify_active = 1;
+ wake_up_all(&ctx->stop_wq);
+ smp_wmb();
+ }
+ mutex_unlock(&spu_prio->active_mutex[node]);
+ }
+ yield();
+}
+
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)
@@ -250,6 +277,14 @@
return spu_get_idle(ctx, flags);
}

+void spu_notify_already_active(struct spu_context *ctx)
+{
+ struct spu *spu = ctx->spu;
+ if (!spu)
+ return;
+ spu_switch_notify(spu, ctx);
+}
+
/* The three externally callable interfaces
* for the scheduler begin here.
*
Index: linux-2.6.19-rc6-arnd1+patches/arch/powerpc/platforms/cell/spufs/spufs.h
===================================================================
--- linux-2.6.19-rc6-arnd1+patches.orig/arch/powerpc/platforms/cell/spufs/spufs.h 2007-01-08 18:18:40.093354608 -0600
+++ linux-2.6.19-rc6-arnd1+patches/arch/powerpc/platforms/cell/spufs/spufs.h 2007-01-08 18:31:03.610345792 -0600
@@ -183,6 +183,7 @@
void spu_yield(struct spu_context *ctx);
int __init spu_sched_init(void);
void __exit spu_sched_exit(void);
+void spu_notify_already_active(struct spu_context *ctx);

extern char *isolated_loader;

Index: linux-2.6.19-rc6-arnd1+patches/arch/powerpc/platforms/cell/spufs/run.c
===================================================================
--- linux-2.6.19-rc6-arnd1+patches.orig/arch/powerpc/platforms/cell/spufs/run.c 2007-01-08 18:33:51.979311680 -0600
+++ linux-2.6.19-rc6-arnd1+patches/arch/powerpc/platforms/cell/spufs/run.c 2007-01-15 16:31:30.104499992 -0600
@@ -45,9 +45,11 @@
u64 pte_fault;

*stat = ctx->ops->status_read(ctx);
- if (ctx->state != SPU_STATE_RUNNABLE)
- return 1;
+ smp_rmb();
+
spu = ctx->spu;
+ if (ctx->state != SPU_STATE_RUNNABLE || spu->notify_active)
+ return 1;
pte_fault = spu->dsisr &
(MFC_DSISR_PTE_NOT_FOUND | MFC_DSISR_ACCESS_DENIED);
return (!(*stat & 0x1) || pte_fault || spu->class_0_pending) ? 1 : 0;
@@ -304,6 +306,7 @@
u32 *npc, u32 *event)
{
int ret;
+ struct * spu;
u32 status;

if (down_interruptible(&ctx->run_sema))
@@ -317,8 +320,16 @@

do {
ret = spufs_wait(ctx->stop_wq, spu_stopped(ctx, &status));
+ spu = ctx->spu;
if (unlikely(ret))
break;
+ if (unlikely(spu->notify_active)) {
+ spu->notify_active = 0;
+ if (!(status & SPU_STATUS_STOPPED_BY_STOP)) {
+ spu_notify_already_active(ctx);
+ continue;
+ }
+ }
if ((status & SPU_STATUS_STOPPED_BY_STOP) &&
(status >> SPU_STOP_STATUS_SHIFT == 0x2104)) {
ret = spu_process_callback(ctx);


Relevant Pages

  • Re: [PATCH] kvm: remove in_range from kvm_io_device
    ... as well as better error handling by ... Hi Michael, ... Short term it might be also possible to go back to keeping kvm lock ... The patch that adds value to in_range is still okay. ...
    (Linux-Kernel)
  • Re: serious word template issue in Word 2003
    ... confirm that patch KB887979 is the culprit. ... >> template's dialog box, I'm going to use that to enable a template ... >>> to see if they can provide a better way to fix this problem. ...
    (microsoft.public.office.developer.vba)
  • Re: [patch] pcspkr: fix dependancies
    ... patch completely, or revert the dependancies ... like Michael suggested initially (if I ... embedded devices that we could omit this enumeration ... Free Embedded Linux Training Materials ...
    (Linux-Kernel)
  • Re: Ingres 2.6 buggy
    ... Michael Martin wrote: ... When CA fixed something in a patch, ... the importance of testing a new patch or version prior to installing it ... unless of course it is extremely urgent and the unknown risk of the ...
    (comp.databases.ingres)
  • Re: [Cbe-oss-dev] [PATCH] Cell SPU task notification - repost of patch with updates
    ... This patch makes all the changes suggested by Christopher, with the exception of the suggestion to use the sched_flags field versus a new member to the spu_context struct to signal the need to "notify already active". ... Enable SPU switch notification to detect currently active SPU tasks. ...
    (Linux-Kernel)