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