Re: [PATCH 0/5] make *_gate_vma accept mm_struct instead of task_struct
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. This is the first of a two part series that implements safe writes to /proc/pid/mem. I will be posting the second series to lkml shortly. These Making every syscall slower for /proc/pid/mem doesn't seem like a good tradeoff to me. Please solve this in some other way. -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
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
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. I do not think this will result in cache misses on the scale you suggest. I am simply mirroring the *state* of the TIF_IA32 flag in mm_struct, not testing/accessing it in the same way. The only place where this flag is accessed (outside the exec() syscall path) is in x86/mm/init_64.c, get_gate_vma(), which in turn is needed by a few, relatively heavy weight, page locking/pinning routines on the mm side (get_user_pages, for example). Patches 3 and 4 in the series show the extent of the change. Or am I missing something? /proc/pid/mem. I will be posting the second series to lkml shortly. These Making every syscall slower for /proc/pid/mem doesn't seem like a good tradeoff to me. Please solve this in some other way. -Andi -- 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
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
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
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
Re: [PATCH 0/5] make *_gate_vma accept mm_struct instead of task_struct
TIF_IA32 is set during the execution of a 32-bit system call - so touched on each compat system call. Is this the actual flag you want? A 32-bit address space flag is different from TIF_IA32. -- Sent from my mobile phone. Please pardon any lack of formatting. Stephen Wilson wils...@start.ca 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: 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. I do not think this will result in cache misses on the scale you suggest. I am simply mirroring the *state* of the TIF_IA32 flag in mm_struct, not testing/accessing it in the same way. The only place where this flag is accessed (outside the exec() syscall path) is in x86/mm/init_64.c, get_gate_vma(), which in turn is needed by a few, relatively heavy weight, page locking/pinning routines on the mm side (get_user_pages, for example). Patches 3 and 4 in the series show the extent of the change. Or am I missing something? /proc/pid/mem. I will be posting the second series to lkml shortly. These Making every syscall slower for /proc/pid/mem doesn't seem like a good tradeoff to me. Please solve this in some other way. -Andi -- 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
On Wed, Mar 09, 2011 at 05:09:09AM -0800, Michel Lespinasse wrote: On Tue, Mar 8, 2011 at 4:31 PM, Stephen Wilson wils...@start.ca 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. Reviewed-by: Michel Lespinasse wal...@google.com May I suggest ia32_compat instead of just compat for the flag name ? Yes, sounds good to me. Will change in the next iteration. Thanks for the review! -- Michel Walken Lespinasse A program is never fully debugged until the last user dies. -- 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
On Tue, Mar 8, 2011 at 4:31 PM, Stephen Wilson wils...@start.ca 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. Reviewed-by: Michel Lespinasse wal...@google.com May I suggest ia32_compat instead of just compat for the flag name ? -- Michel Walken Lespinasse A program is never fully debugged until the last user dies. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/5] make *_gate_vma accept mm_struct instead of task_struct
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. This is the first of a two part series that implements safe writes to /proc/pid/mem. I will be posting the second series to lkml shortly. These patches are based on v2.6.38-rc8. The general approach used here was suggested to me by Alexander Viro, but any mistakes present in these patches are entirely my own. -- steve Stephen Wilson (5): x86: add context tag to mark mm when running a task in 32-bit compatibility mode x86: mark associated mm when running a task in 32 bit compatibility mode mm: arch: make get_gate_vma take an mm_struct instead of a task_struct mm: arch: make in_gate_area take an mm_struct instead of a task_struct mm: arch: rename in_gate_area_no_task to in_gate_area_no_mm arch/powerpc/kernel/vdso.c |6 +++--- arch/s390/kernel/vdso.c|6 +++--- arch/sh/kernel/vsyscall/vsyscall.c |6 +++--- arch/x86/ia32/ia32_aout.c |1 + arch/x86/include/asm/mmu.h |6 ++ arch/x86/kernel/process_64.c |8 arch/x86/mm/init_64.c | 16 arch/x86/vdso/vdso32-setup.c | 15 --- fs/binfmt_elf.c|2 +- fs/proc/task_mmu.c |8 +--- include/linux/mm.h | 10 +- kernel/kallsyms.c |4 ++-- mm/memory.c|8 mm/mlock.c |4 ++-- mm/nommu.c |2 +- 15 files changed, 61 insertions(+), 42 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
