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

[Luke B. <Luk...@us...>]

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

>
> Subject: Enable SPU switch notification to detect currently active SPU
tasks.
>
> From: Maynard Johnson <may...@us...>
>
> 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 <mp...@us...>
>
>
> 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