lkml.org 
[lkml]   [2007]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Cbe-oss-dev] [RFC, PATCH 3/4] Add support to OProfile for profiling Cell BE SPUs -- update
    Arnd Bergmann wrote:

    > On Monday 29 January 2007 20:48, Maynard Johnson wrote:
    >
    >>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 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@us.ibm.com>
    >
    >
    > I looked through it again, and think I found a serious bug, but that
    > should be easy enough to solve:
    >
    >
    >>+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_switch_notify();
    >>+ */
    >>+ 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;
    >
    >
    > [side note]
    > There is a small whitespace breakage in here, please make sure you always
    > use tabs for indenting, not space characters.
    > [/side note]
    >
    >
    >>@@ -45,9 +45,10 @@
    >> u64 pte_fault;
    >>
    >> *stat = ctx->ops->status_read(ctx);
    >>- if (ctx->state != SPU_STATE_RUNNABLE)
    >>- return 1;
    >>+
    >> 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;
    >>@@ -305,6 +306,7 @@
    >> u32 *npc, u32 *event)
    >> {
    >> int ret;
    >>+ struct spu * spu;
    >> u32 status;
    >>
    >> if (down_interruptible(&ctx->run_sema))
    >>@@ -318,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_switch_notify(spu, ctx);
    >>+ continue;
    >>+ }
    >>+ }
    >
    >
    > This is before spu_reacquire_runnable, so in case the spu got
    > preempted at the same time when oprofile was enabled, ctx->spu
    > is NULL, and you can't load the notify_active flag from it.
    >
    > On solution would be to move the notify_active flag from ctx->spu
    > into ctx itself, but maybe there are other ways to solve this.
    In an earlier review of this patch, Christopher Hellwig suggested I move
    the notify_active flag to be a bit in the sched_flags field that's added
    in his scheduler patch series. If this patch series will be a available
    in an "Arnd" tree that we'll be using for our current OProfile
    development, perhaps I should wait until that time to change this, since
    the window of vulnerability is quite small. What do you think?

    -Maynard
    >
    > Thanks,
    >
    > Arnd <><


    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-01-30 16:35    [W:0.029 / U:30.184 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site