All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maynard Johnson <maynardj@us.ibm.com>
To: Arnd Bergmann <arnd.bergmann@de.ibm.com>
Cc: maynardj@linux.vnet.ibm.com, Luke Browning <lukeb@br.ibm.com>,
	linuxppc-dev-bounces+lukebrowning=us.ibm.com@ozlabs.org,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	oprofile-list@lists.sourceforge.net, cbe-oss-dev@ozlabs.org
Subject: Re: [Cbe-oss-dev] [PATCH]Add notification for active Cell SPU tasks
Date: Fri, 08 Dec 2006 09:04:30 -0600	[thread overview]
Message-ID: <45797EFE.3040008@us.ibm.com> (raw)
In-Reply-To: <200612072358.15707.arnd.bergmann@de.ibm.com>

Arnd Bergmann wrote:

>On Wednesday 06 December 2006 23:04, Maynard Johnson wrote:
>  
>
>>text(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
>>>>        
>>>>
>>I could be wrong, but I think if we moved the mutex_lock to be inside of 
>>the list_for_each_entry loop, we could have a race condition.  For 
>>example, we obtain the next spu item from the spu_prio->active_mutex 
>>list, then wait on the mutex which is being held for the purpose of 
>>removing the very spu context we just obtained.
>>
>>    
>>
>>>>every context
>>>>switch.  Can't oprofile internally derive the list of active spus 
>>>>        
>>>>
>>>from the  
>>>      
>>>
>>>>context switch callout. 
>>>>        
>>>>
>>Arnd would certainly know the answer to this off the top of his head, 
>>but when I initially discussed the idea for this patch with him 
>>(probably a couple months ago or so), he didn't suggest a better 
>>alternative.  Perhaps there is a way to do this with current SPUFS 
>>code.  Arnd, any comments on this?
>>    
>>
>
>
>
>No code should ever need to look at other SPUs when performing an
>operation on a given SPU, so we don't need to hold a global lock
>during normal operation.
>
>We have two cases that need to be handled:
>
>- on each context unload and load (both for a full switch operation),
>  call to the profiling code with a pointer to the current context
>  and spu (context is NULL when unloading).
>
>  If the new context is not know yet, scan its overlay table (expensive)
>  and store information about it in an oprofile private object. Otherwise
>  just point to the currently active object, this should be really cheap.
>
>- When enabling oprofile initially, scan all contexts that are currently
>  running on one of the SPUs. This is also expensive, but should happen
>  before the measurement starts so it does not impact the resulting data.
>
>  
>
>>>>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. 
>>>>        
>>>>
>>I'm not exactly sure what you're saying here.  Are you suggesting that a 
>>user may only be interested in acitve SPU notification and, therefore, 
>>shouldn't have to be depenent on the "standard" notification 
>>registration succeeding?  There may be a case for adding a new 
>>registration function, I suppose; although, I'm not aware of any other 
>>users of the SPUFS notification mechanism besides OProfile and PDT, and 
>>we need notification of both active and future SPU tasks.  But I would 
>>not object to a new function.
>>
>>    
>>
>I think what Luke was trying to get to is that notify_spus_active() should
>not call blocking_notifier_call_chain(), since it will notify other users
>as well as the newly registered one. Instead, it can simply call the
>notifier function directly.
>  
>
Ah, yes.  Thanks to both of you for pointing that out.  I'll fix that 
and re-post.

-Maynard

>	Arnd <><
>
>-------------------------------------------------------------------------
>Take Surveys. Earn Cash. Influence the Future of IT
>Join SourceForge.net's Techsay panel and you'll get the chance to share your
>opinions on IT & business topics through brief surveys - and earn cash
>http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>_______________________________________________
>oprofile-list mailing list
>oprofile-list@lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/oprofile-list
>  
>

WARNING: multiple messages have this Message-ID (diff)
From: Maynard Johnson <maynardj@us.ibm.com>
To: Arnd Bergmann <arnd.bergmann@de.ibm.com>
Cc: cbe-oss-dev@ozlabs.org, maynardj@linux.vnet.ibm.com,
	Luke Browning <lukeb@br.ibm.com>,
	linuxppc-dev-bounces+lukebrowning=us.ibm.com@ozlabs.org,
	Luke Browning <lukebrowning@us.ibm.com>,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	oprofile-list@lists.sourceforge.net
Subject: Re: [Cbe-oss-dev] [PATCH]Add notification for active Cell SPU tasks
Date: Fri, 08 Dec 2006 09:04:30 -0600	[thread overview]
Message-ID: <45797EFE.3040008@us.ibm.com> (raw)
In-Reply-To: <200612072358.15707.arnd.bergmann@de.ibm.com>

Arnd Bergmann wrote:

>On Wednesday 06 December 2006 23:04, Maynard Johnson wrote:
>  
>
>>text(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
>>>>        
>>>>
>>I could be wrong, but I think if we moved the mutex_lock to be inside of 
>>the list_for_each_entry loop, we could have a race condition.  For 
>>example, we obtain the next spu item from the spu_prio->active_mutex 
>>list, then wait on the mutex which is being held for the purpose of 
>>removing the very spu context we just obtained.
>>
>>    
>>
>>>>every context
>>>>switch.  Can't oprofile internally derive the list of active spus 
>>>>        
>>>>
>>>from the  
>>>      
>>>
>>>>context switch callout. 
>>>>        
>>>>
>>Arnd would certainly know the answer to this off the top of his head, 
>>but when I initially discussed the idea for this patch with him 
>>(probably a couple months ago or so), he didn't suggest a better 
>>alternative.  Perhaps there is a way to do this with current SPUFS 
>>code.  Arnd, any comments on this?
>>    
>>
>
>
>
>No code should ever need to look at other SPUs when performing an
>operation on a given SPU, so we don't need to hold a global lock
>during normal operation.
>
>We have two cases that need to be handled:
>
>- on each context unload and load (both for a full switch operation),
>  call to the profiling code with a pointer to the current context
>  and spu (context is NULL when unloading).
>
>  If the new context is not know yet, scan its overlay table (expensive)
>  and store information about it in an oprofile private object. Otherwise
>  just point to the currently active object, this should be really cheap.
>
>- When enabling oprofile initially, scan all contexts that are currently
>  running on one of the SPUs. This is also expensive, but should happen
>  before the measurement starts so it does not impact the resulting data.
>
>  
>
>>>>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. 
>>>>        
>>>>
>>I'm not exactly sure what you're saying here.  Are you suggesting that a 
>>user may only be interested in acitve SPU notification and, therefore, 
>>shouldn't have to be depenent on the "standard" notification 
>>registration succeeding?  There may be a case for adding a new 
>>registration function, I suppose; although, I'm not aware of any other 
>>users of the SPUFS notification mechanism besides OProfile and PDT, and 
>>we need notification of both active and future SPU tasks.  But I would 
>>not object to a new function.
>>
>>    
>>
>I think what Luke was trying to get to is that notify_spus_active() should
>not call blocking_notifier_call_chain(), since it will notify other users
>as well as the newly registered one. Instead, it can simply call the
>notifier function directly.
>  
>
Ah, yes.  Thanks to both of you for pointing that out.  I'll fix that 
and re-post.

-Maynard

>	Arnd <><
>
>-------------------------------------------------------------------------
>Take Surveys. Earn Cash. Influence the Future of IT
>Join SourceForge.net's Techsay panel and you'll get the chance to share your
>opinions on IT & business topics through brief surveys - and earn cash
>http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>_______________________________________________
>oprofile-list mailing list
>oprofile-list@lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/oprofile-list
>  
>



  reply	other threads:[~2006-12-08 15:04 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-01 20:01 [PATCH]Add notification for active Cell SPU tasks Maynard Johnson
2006-12-02 20:00 ` [Cbe-oss-dev] " Arnd Bergmann
2006-12-02 20:00   ` Arnd Bergmann
2006-12-04 15:36   ` Maynard Johnson
2006-12-04 15:36     ` Maynard Johnson
2006-12-04 12:26 ` Luke Browning
2006-12-06 19:30   ` Luke Browning
2006-12-06 22:04     ` Maynard Johnson
2006-12-06 22:04       ` Maynard Johnson
2006-12-07 22:58       ` [Cbe-oss-dev] " Arnd Bergmann
2006-12-07 22:58         ` Arnd Bergmann
2006-12-08 15:04         ` Maynard Johnson [this message]
2006-12-08 15:04           ` Maynard Johnson
2006-12-08 19:11           ` Luke Browning
2006-12-12 14:47             ` Maynard Johnson
2006-12-12 14:47               ` Maynard Johnson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45797EFE.3040008@us.ibm.com \
    --to=maynardj@us.ibm.com \
    --cc=arnd.bergmann@de.ibm.com \
    --cc=cbe-oss-dev@ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev-bounces+lukebrowning=us.ibm.com@ozlabs.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=lukeb@br.ibm.com \
    --cc=maynardj@linux.vnet.ibm.com \
    --cc=oprofile-list@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.