Re: [PATCH 0/5] make *_gate_vma accept mm_struct instead of task_struct II

2011-03-10 Thread Andi Kleen
On Thu, Mar 10, 2011 at 08:00:32AM -0800, Andi Kleen wrote:
 On Tue, Mar 08, 2011 at 07:31:56PM -0500, Stephen Wilson wrote:
  
  Morally, the question of whether an address lies in a gate vma should be 
  asked
  with respect to an mm, not a particular task.
  
  Practically, dropping the dependency on task_struct will help make current 
  and
  future operations on mm's more flexible and convenient.  In particular, it
  allows some code paths to avoid the need to hold task_lock.
  
  The only architecture this change impacts in any significant way is x86_64.
  The principle change on that architecture is to mirror TIF_IA32 via
  a new flag in mm_context_t. 
 
 The problem is -- you're adding a likely cache miss on mm_struct for
 every 32bit compat syscall now, even if they don't need mm_struct
 currently (and a lot of them do not) Unless there's a very good
 justification to make up for this performance issue elsewhere
 (including numbers) this seems like a bad idea.

Hmm I see you're only setting it on exec time actually on rereading
the patches. I thought you were changing TS_COMPAT which is in
the syscall path.

Never mind.  I have no problems with doing such a change on exec
time.

-Andi
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 0/5] make *_gate_vma accept mm_struct instead of task_struct II

2011-03-10 Thread Stephen Wilson

On Thu, Mar 10, 2011 at 08:38:09AM -0800, Andi Kleen wrote:
 On Thu, Mar 10, 2011 at 08:00:32AM -0800, Andi Kleen wrote:
  On Tue, Mar 08, 2011 at 07:31:56PM -0500, Stephen Wilson wrote:
   The only architecture this change impacts in any significant way is 
   x86_64.
   The principle change on that architecture is to mirror TIF_IA32 via
   a new flag in mm_context_t. 
  
  The problem is -- you're adding a likely cache miss on mm_struct for
  every 32bit compat syscall now, even if they don't need mm_struct
  currently (and a lot of them do not) Unless there's a very good
  justification to make up for this performance issue elsewhere
  (including numbers) this seems like a bad idea.
 
 Hmm I see you're only setting it on exec time actually on rereading
 the patches. I thought you were changing TS_COMPAT which is in
 the syscall path.
 
 Never mind.  I have no problems with doing such a change on exec
 time.

OK.  Great!  Does this mean I have your ACK'ed by or reviewed by?


Thanks for taking a look!

-- 
steve

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 0/5] make *_gate_vma accept mm_struct instead of task_struct II

2011-03-10 Thread Andi Kleen
On Thu, Mar 10, 2011 at 11:54:14AM -0500, Stephen Wilson wrote:
 
 On Thu, Mar 10, 2011 at 08:38:09AM -0800, Andi Kleen wrote:
  On Thu, Mar 10, 2011 at 08:00:32AM -0800, Andi Kleen wrote:
   On Tue, Mar 08, 2011 at 07:31:56PM -0500, Stephen Wilson wrote:
The only architecture this change impacts in any significant way is 
x86_64.
The principle change on that architecture is to mirror TIF_IA32 via
a new flag in mm_context_t. 
   
   The problem is -- you're adding a likely cache miss on mm_struct for
   every 32bit compat syscall now, even if they don't need mm_struct
   currently (and a lot of them do not) Unless there's a very good
   justification to make up for this performance issue elsewhere
   (including numbers) this seems like a bad idea.
  
  Hmm I see you're only setting it on exec time actually on rereading
  the patches. I thought you were changing TS_COMPAT which is in
  the syscall path.
  
  Never mind.  I have no problems with doing such a change on exec
  time.
 
 OK.  Great!  Does this mean I have your ACK'ed by or reviewed by?

I didn't read it all, but the first two patches looked ok.

-Andi
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 0/5] make *_gate_vma accept mm_struct instead of task_struct II

2011-03-10 Thread H. Peter Anvin
Sorry... I confused them too. It's TS_COMPAT which is problematic.
-- 
Sent from my mobile phone. Please pardon any lack of formatting.

Stephen Wilson wils...@start.ca wrote:

On Thu, Mar 10, 2011 at 08:38:09AM -0800, Andi Kleen wrote:  On Thu, Mar 10, 
2011 at 08:00:32AM -0800, Andi Kleen wrote:   On Tue, Mar 08, 2011 at 
07:31:56PM -0500, Stephen Wilson wrote:The only architecture this change 
impacts in any significant way is x86_64.The principle change on that 
architecture is to mirror TIF_IA32 viaa new flag in mm_context_t.
 The problem is -- you're adding a likely cache miss on mm_struct for   
every 32bit compat syscall now, even if they don't need mm_struct   currently 
(and a lot of them do not) Unless there's a very good   justification to make 
up for this performance issue elsewhere   (including numbers) this seems like 
a bad idea.   Hmm I see you're only setting it on exec time actually on 
rereading  the patches. I thought you were changing TS_COMPAT which is in  
the syscall path.   Never mind. I have no problems with doing such a change 
on exec  time. OK. Great! Does this mean I have your ACK'e!
 d by or
reviewed by? Thanks for taking a look! -- steve 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev