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