2007-02-03 23:48:59

by Maynard Johnson

[permalink] [raw]
Subject: Re: [Cbe-oss-dev] [RFC, PATCH 4/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: Add support to OProfile for profiling Cell BE SPUs
>>
>>
>>
>
>
[snip]

>>+ *
>>+ * Ideally, we would like to be able to create the cached_info for
>>+ * an SPU task just one time -- when libspe first loads the SPU
>>+ * binary file. We would store the cached_info in a list. Then, as
>>+ * SPU tasks are switched out and new ones switched in, the cached_info
>>+ * for inactive tasks would be kept, and the active one would be placed
>>+ * at the head of the list. But this technique may not with
>>+ * current spufs functionality since the spu used in bind_context may
>>+ * be a different spu than was used in a previous bind_context for a
>>+ * reactivated SPU task. Additionally, a reactivated SPU task may be
>>+ * assigned to run on a different physical SPE. We will investigate
>>+ * further if this can be done.
>>+ *
>>+ */
>>
>>
>
>You should stuff a pointer to cached_info into struct spu_context,
>e.g. 'void *profile_private'.
>
>
I seem to recall looking at this option a while back, but didn't go that
route since struct spu_context is opaque to me. With such a teqnique, I
could then use a simple 16-element array of pointers to cached_info
objects, creating them as needed when spu_context->profile_private is
NULL. I suppose the better option for now is to add a
get_profile_private() function to SPUFs, rather than requiring
spu_context to be visible. Don't know why I didn't think to do that
before. Ah, well, live and learn.

-Maynard

>
>
>>+struct cached_info {
>>+ vma_map_t * map;
>>+ struct spu * the_spu;
>>+ struct kref cache_ref;
>>+ struct list_head list;
>>+};
>>
>>
>
>And replace the 'the_spu' member with a back pointer to the
>spu_context if you need it.
>
>
>
> Arnd <><
>
>



2007-02-04 02:52:14

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [Cbe-oss-dev] [RFC, PATCH 4/4] Add support to OProfile for profiling Cell BE SPUs -- update

On Sunday 04 February 2007 00:49, Maynard Johnson wrote:
> I seem to recall looking at this option a while back, but didn't go that
> route since struct spu_context is opaque to me. ?With such a teqnique, I
> could then use a simple 16-element array of ?pointers to cached_info
> objects, creating them as needed when spu_context->profile_private is
> NULL. ?I suppose the better option for now is to add a
> get_profile_private() function to SPUFs, rather than requiring
> spu_context to be visible.

Yes, that sounds good. Note that the file providing the
spufs_get_profile_private (and respective spufs_set_profile_private)
functions needs to be compiled into the kernel then in case oprofile
gets linked in but spufs is a module.

I think it would also be necessary to have another interface for cleaning
up this data when spufs destroys the context. That could possibly
a variation of the existing notifier call, or a new call, or you
establish the convention that if the private pointer is non-NULL,
spufs will kfree it.

Arnd <><

2007-02-04 17:33:45

by Maynard Johnson

[permalink] [raw]
Subject: Re: [Cbe-oss-dev] [RFC, PATCH 4/4] Add support to OProfile for profiling Cell BE SPUs -- update

Arnd Bergmann wrote:

>On Sunday 04 February 2007 00:49, Maynard Johnson wrote:
>
>
>>I seem to recall looking at this option a while back, but didn't go that
>>route since struct spu_context is opaque to me. With such a teqnique, I
>>could then use a simple 16-element array of pointers to cached_info
>>objects, creating them as needed when spu_context->profile_private is
>>NULL. I suppose the better option for now is to add a
>>get_profile_private() function to SPUFs, rather than requiring
>>spu_context to be visible.
>>
>>
>
>Yes, that sounds good. Note that the file providing the
>spufs_get_profile_private (and respective spufs_set_profile_private)
>functions needs to be compiled into the kernel then in case oprofile
>gets linked in but spufs is a module.
>
>
Hmm . . . we already depend on the register/unregister functions in
sched.c, so my patch changes the oprofile Kconfig to default to 'm' and
'depends on SPU_FS'.

>I think it would also be necessary to have another interface for cleaning
>up this data when spufs destroys the context. That could possibly
>a variation of the existing notifier call, or a new call, or you
>establish the convention that if the private pointer is non-NULL,
>spufs will kfree it.
>
>
Yes, I was thnking along the lines of your last suggestion. I presume
OProfile gets notified (object_id == 0) before the context is actually
destroyed. At that time, we would NULL-out the reference to the
cached_info, so then SPUFS would kfree it at destroy time.

-Maynard

> Arnd <><
>
>