* getxpid() parent lookup is broken
@ 2010-09-26 4:34 Ben Hutchings
2012-05-29 4:55 ` Matt Turner
0 siblings, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2010-09-26 4:34 UTC (permalink / raw)
To: linux-alpha
[-- Attachment #1: Type: text/plain, Size: 1244 bytes --]
I noticed that Tomoyo doesn't build on Alpha because Tomoyo tries to
call sys_getpid() and sys_getppid(). That's a bug in Tomoyo, but when I
looked at why Alpha is different I found that the implementation of
getxpid() hasn't been kept in sync getppid() for other architectures and
is presumably now incorrect.
sys_getppid():
rcu_read_lock();
pid = task_tgid_vnr(current->real_parent);
rcu_read_unlock();
task_tgid_vnr(current->real_parent) expands through various inline
functions to:
pid_nr_ns(current->real_parent->group_leader->pids[PIDTYPE_PID].pid,
current->nsproxy->pid_ns)
sys_getxpid():
/* See linux/kernel/timer.c sys_getppid for discussion
about this loop. */
ldq $3, TASK_GROUP_LEADER($2)
ldq $4, TASK_REAL_PARENT($3)
ldl $0, TASK_TGID($2)
1: ldl $1, TASK_TGID($4)
#ifdef CONFIG_SMP
mov $4, $5
mb
ldq $3, TASK_GROUP_LEADER($2)
ldq $4, TASK_REAL_PARENT($3)
cmpeq $4, $5, $5
beq $5, 1b
#endif
The comment is obviously out-of-date. This isn't following RCU protocol
and it isn't namespace-aware. I think it needs to be turned into a
wrapper for the generic code.
Ben.
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: getxpid() parent lookup is broken 2010-09-26 4:34 getxpid() parent lookup is broken Ben Hutchings @ 2012-05-29 4:55 ` Matt Turner 2012-05-29 14:09 ` Al Viro 0 siblings, 1 reply; 4+ messages in thread From: Matt Turner @ 2012-05-29 4:55 UTC (permalink / raw) To: Ben Hutchings, Al Viro; +Cc: linux-alpha, Tobias Klausmann, Michael Cree On Sun, Sep 26, 2010 at 12:34 AM, Ben Hutchings <ben@decadent.org.uk> wrote: > I noticed that Tomoyo doesn't build on Alpha because Tomoyo tries to > call sys_getpid() and sys_getppid(). That's a bug in Tomoyo, but when I > looked at why Alpha is different I found that the implementation of > getxpid() hasn't been kept in sync getppid() for other architectures and > is presumably now incorrect. > > sys_getppid(): > rcu_read_lock(); > pid = task_tgid_vnr(current->real_parent); > rcu_read_unlock(); > > task_tgid_vnr(current->real_parent) expands through various inline > functions to: > > pid_nr_ns(current->real_parent->group_leader->pids[PIDTYPE_PID].pid, > current->nsproxy->pid_ns) > > sys_getxpid(): > /* See linux/kernel/timer.c sys_getppid for discussion > about this loop. */ > ldq $3, TASK_GROUP_LEADER($2) > ldq $4, TASK_REAL_PARENT($3) > ldl $0, TASK_TGID($2) > 1: ldl $1, TASK_TGID($4) > #ifdef CONFIG_SMP > mov $4, $5 > mb > ldq $3, TASK_GROUP_LEADER($2) > ldq $4, TASK_REAL_PARENT($3) > cmpeq $4, $5, $5 > beq $5, 1b > #endif > > The comment is obviously out-of-date. This isn't following RCU protocol > and it isn't namespace-aware. I think it needs to be turned into a > wrapper for the generic code. > > Ben. > > -- > Ben Hutchings > Once a job is fouled up, anything done to improve it makes it worse. Hi Ben, Al, I recently looked at this gentoo bug -- https://bugs.gentoo.org/show_bug.cgi?id=405829 and then came across this email and the two patches. They seem possibly related. It looks like Al said he thought he had a better way of fixing the problem, but then I'm not sure if his patches surfaced or not. Also possibly related, Tobias and Michael have seen some RCU stalls with recent kernels. Looks like getxpid needs an update? Thanks, Matt -- To unsubscribe from this list: send the line "unsubscribe linux-alpha" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: getxpid() parent lookup is broken 2012-05-29 4:55 ` Matt Turner @ 2012-05-29 14:09 ` Al Viro 2012-05-29 14:18 ` Al Viro 0 siblings, 1 reply; 4+ messages in thread From: Al Viro @ 2012-05-29 14:09 UTC (permalink / raw) To: Matt Turner; +Cc: Ben Hutchings, linux-alpha, Tobias Klausmann, Michael Cree On Tue, May 29, 2012 at 12:55:10AM -0400, Matt Turner wrote: > I recently looked at this gentoo bug -- > https://bugs.gentoo.org/show_bug.cgi?id=405829 and then came across > this email and the two patches. They seem possibly related. > > It looks like Al said he thought he had a better way of fixing the > problem, but then I'm not sure if his patches surfaced or not. > > Also possibly related, Tobias and Michael have seen some RCU stalls > with recent kernels. Looks like getxpid needs an update? Umm... Let me see if I can find it... Here: alpha: take a bunch of syscalls into osf_sys.c New helper: current_thread_info(). Allows to do a bunch of odd syscalls in C. While we are at it, there had never been a reason to do osf_getpriority() in assembler. We also get "namespace"-aware (read: consistent with getuid(2), etc.) behaviour from getx?id() syscalls now. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> diff --git a/arch/alpha/include/asm/ptrace.h b/arch/alpha/include/asm/ptrace.h index fd698a1..b87755a 100644 --- a/arch/alpha/include/asm/ptrace.h +++ b/arch/alpha/include/asm/ptrace.h @@ -76,7 +76,10 @@ struct switch_stack { #define task_pt_regs(task) \ ((struct pt_regs *) (task_stack_page(task) + 2*PAGE_SIZE) - 1) -#define force_successful_syscall_return() (task_pt_regs(current)->r0 = 0) +#define current_pt_regs() \ + ((struct pt_regs *) ((char *)current_thread_info() + 2*PAGE_SIZE) - 1) + +#define force_successful_syscall_return() (current_pt_regs()->r0 = 0) #endif diff --git a/arch/alpha/kernel/entry.S b/arch/alpha/kernel/entry.S index 6d159ce..22b0c4d 100644 --- a/arch/alpha/kernel/entry.S +++ b/arch/alpha/kernel/entry.S @@ -797,115 +797,6 @@ sys_rt_sigreturn: .end sys_rt_sigreturn .align 4 - .globl sys_sethae - .ent sys_sethae -sys_sethae: - .prologue 0 - stq $16, 152($sp) - ret -.end sys_sethae - - .align 4 - .globl osf_getpriority - .ent osf_getpriority -osf_getpriority: - lda $sp, -16($sp) - stq $26, 0($sp) - .prologue 0 - - jsr $26, sys_getpriority - - ldq $26, 0($sp) - blt $0, 1f - - /* Return value is the unbiased priority, i.e. 20 - prio. - This does result in negative return values, so signal - no error by writing into the R0 slot. */ - lda $1, 20 - stq $31, 16($sp) - subl $1, $0, $0 - unop - -1: lda $sp, 16($sp) - ret -.end osf_getpriority - - .align 4 - .globl sys_getxuid - .ent sys_getxuid -sys_getxuid: - .prologue 0 - ldq $2, TI_TASK($8) - ldq $3, TASK_CRED($2) - ldl $0, CRED_UID($3) - ldl $1, CRED_EUID($3) - stq $1, 80($sp) - ret -.end sys_getxuid - - .align 4 - .globl sys_getxgid - .ent sys_getxgid -sys_getxgid: - .prologue 0 - ldq $2, TI_TASK($8) - ldq $3, TASK_CRED($2) - ldl $0, CRED_GID($3) - ldl $1, CRED_EGID($3) - stq $1, 80($sp) - ret -.end sys_getxgid - - .align 4 - .globl sys_getxpid - .ent sys_getxpid -sys_getxpid: - .prologue 0 - ldq $2, TI_TASK($8) - - /* See linux/kernel/timer.c sys_getppid for discussion - about this loop. */ - ldq $3, TASK_GROUP_LEADER($2) - ldq $4, TASK_REAL_PARENT($3) - ldl $0, TASK_TGID($2) -1: ldl $1, TASK_TGID($4) -#ifdef CONFIG_SMP - mov $4, $5 - mb - ldq $3, TASK_GROUP_LEADER($2) - ldq $4, TASK_REAL_PARENT($3) - cmpeq $4, $5, $5 - beq $5, 1b -#endif - stq $1, 80($sp) - ret -.end sys_getxpid - - .align 4 - .globl sys_alpha_pipe - .ent sys_alpha_pipe -sys_alpha_pipe: - lda $sp, -16($sp) - stq $26, 0($sp) - .prologue 0 - - mov $31, $17 - lda $16, 8($sp) - jsr $26, do_pipe_flags - - ldq $26, 0($sp) - bne $0, 1f - - /* The return values are in $0 and $20. */ - ldl $1, 12($sp) - ldl $0, 8($sp) - - stq $1, 80+16($sp) -1: lda $sp, 16($sp) - ret -.end sys_alpha_pipe - - .align 4 .globl sys_execve .ent sys_execve sys_execve: diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c index 49ee319..3c4e16a 100644 --- a/arch/alpha/kernel/osf_sys.c +++ b/arch/alpha/kernel/osf_sys.c @@ -1238,3 +1238,52 @@ SYSCALL_DEFINE3(osf_writev, unsigned long, fd, } #endif + +SYSCALL_DEFINE2(osf_getpriority, int, which, int, who) +{ + int prio = sys_getpriority(which, who); + if (prio >= 0) { + /* Return value is the unbiased priority, i.e. 20 - prio. + This does result in negative return values, so signal + no error */ + force_successful_syscall_return(); + prio = 20 - prio; + } + return prio; +} + +SYSCALL_DEFINE0(getxuid) +{ + current_pt_regs()->r20 = sys_geteuid(); + return sys_getuid(); +} + +SYSCALL_DEFINE0(getxgid) +{ + current_pt_regs()->r20 = sys_getegid(); + return sys_getgid(); +} + +SYSCALL_DEFINE0(getxpid) +{ + current_pt_regs()->r20 = sys_getppid(); + return sys_getpid(); +} + +SYSCALL_DEFINE0(alpha_pipe) +{ + int fd[2]; + int res = do_pipe_flags(fd, 0); + if (!res) { + /* The return values are in $0 and $20. */ + current_pt_regs()->r20 = fd[1]; + res = fd[0]; + } + return res; +} + +SYSCALL_DEFINE1(sethae, unsigned long, val) +{ + current_pt_regs()->hae = val; + return 0; +} diff --git a/arch/alpha/kernel/systbls.S b/arch/alpha/kernel/systbls.S index e534e1c..97c9030 100644 --- a/arch/alpha/kernel/systbls.S +++ b/arch/alpha/kernel/systbls.S @@ -111,7 +111,7 @@ sys_call_table: .quad sys_socket .quad sys_connect .quad sys_accept - .quad osf_getpriority /* 100 */ + .quad sys_osf_getpriority /* 100 */ .quad sys_send .quad sys_recv .quad sys_sigreturn diff --git a/kernel/timer.c b/kernel/timer.c index 6ec7e7e..6885e5f 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -1394,13 +1394,6 @@ SYSCALL_DEFINE1(alarm, unsigned int, seconds) #endif -#ifndef __alpha__ - -/* - * The Alpha uses getxpid, getxuid, and getxgid instead. Maybe this - * should be moved into arch/i386 instead? - */ - /** * sys_getpid - return the thread group id of the current process * @@ -1456,8 +1449,6 @@ SYSCALL_DEFINE0(getegid) return from_kgid_munged(current_user_ns(), current_egid()); } -#endif - static void process_timeout(unsigned long __data) { wake_up_process((struct task_struct *)__data); ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: getxpid() parent lookup is broken 2012-05-29 14:09 ` Al Viro @ 2012-05-29 14:18 ` Al Viro 0 siblings, 0 replies; 4+ messages in thread From: Al Viro @ 2012-05-29 14:18 UTC (permalink / raw) To: Matt Turner; +Cc: Ben Hutchings, linux-alpha, Tobias Klausmann, Michael Cree On Tue, May 29, 2012 at 03:09:43PM +0100, Al Viro wrote: > On Tue, May 29, 2012 at 12:55:10AM -0400, Matt Turner wrote: > > > I recently looked at this gentoo bug -- > > https://bugs.gentoo.org/show_bug.cgi?id=405829 and then came across > > this email and the two patches. They seem possibly related. > > > > It looks like Al said he thought he had a better way of fixing the > > problem, but then I'm not sure if his patches surfaced or not. > > > > Also possibly related, Tobias and Michael have seen some RCU stalls > > with recent kernels. Looks like getxpid needs an update? > > Umm... Let me see if I can find it... Here: [snip] BTW, there's another completely untested patch bouncing around in the local tree - takes kernel_execve() out of entry.S diff --git a/arch/alpha/kernel/alpha_ksyms.c b/arch/alpha/kernel/alpha_ksyms.c index d96e742..b77b813 100644 --- a/arch/alpha/kernel/alpha_ksyms.c +++ b/arch/alpha/kernel/alpha_ksyms.c @@ -52,7 +52,6 @@ EXPORT_SYMBOL(alpha_write_fp_reg_s); /* entry.S */ EXPORT_SYMBOL(kernel_thread); -EXPORT_SYMBOL(kernel_execve); /* Networking helper routines. */ EXPORT_SYMBOL(csum_tcpudp_magic); diff --git a/arch/alpha/kernel/entry.S b/arch/alpha/kernel/entry.S index 4074645..a8c9db8 100644 --- a/arch/alpha/kernel/entry.S +++ b/arch/alpha/kernel/entry.S @@ -663,58 +663,6 @@ kernel_thread: br ret_to_kernel .end kernel_thread -/* - * kernel_execve(path, argv, envp) - */ - .align 4 - .globl kernel_execve - .ent kernel_execve -kernel_execve: - /* We can be called from a module. */ - ldgp $gp, 0($27) - lda $sp, -(32+SIZEOF_PT_REGS+8)($sp) - .frame $sp, 32+SIZEOF_PT_REGS+8, $26, 0 - stq $26, 0($sp) - stq $16, 8($sp) - stq $17, 16($sp) - stq $18, 24($sp) - .prologue 1 - - lda $16, 32($sp) - lda $17, 0 - lda $18, SIZEOF_PT_REGS - bsr $26, memset !samegp - - /* Avoid the HAE being gratuitously wrong, which would cause us - to do the whole turn off interrupts thing and restore it. */ - ldq $2, alpha_mv+HAE_CACHE - stq $2, 152+32($sp) - - ldq $16, 8($sp) - ldq $17, 16($sp) - ldq $18, 24($sp) - lda $19, 32($sp) - bsr $26, do_execve !samegp - - ldq $26, 0($sp) - bne $0, 1f /* error! */ - - /* Move the temporary pt_regs struct from its current location - to the top of the kernel stack frame. See copy_thread for - details for a normal process. */ - lda $16, 0x4000 - SIZEOF_PT_REGS($8) - lda $17, 32($sp) - lda $18, SIZEOF_PT_REGS - bsr $26, memmove !samegp - - /* Take that over as our new stack frame and visit userland! */ - lda $sp, 0x4000 - SIZEOF_PT_REGS($8) - br $31, ret_from_sys_call - -1: lda $sp, 32+SIZEOF_PT_REGS+8($sp) - ret -.end kernel_execve - \f /* * Special system calls. Most of these are special in that they either diff --git a/arch/alpha/kernel/process.c b/arch/alpha/kernel/process.c index 153d3fc..d6fde98 100644 --- a/arch/alpha/kernel/process.c +++ b/arch/alpha/kernel/process.c @@ -455,3 +455,22 @@ get_wchan(struct task_struct *p) } return pc; } + +int kernel_execve(const char *path, const char *const argv[], const char *const envp[]) +{ + /* Avoid the HAE being gratuitously wrong, which would cause us + to do the whole turn off interrupts thing and restore it. */ + struct pt_regs regs = {.hae = alpha_mv.hae_cache}; + int err = do_execve(path, argv, envp, ®s); + if (!err) { + struct pt_regs *p = current_pt_regs(); + /* copy regs to normal position and off to userland we go... */ + *p = regs; + __asm__ __volatile__ ( + "mov %0, $sp;" + "br $31, ret_from_sys_call" + : : "r"(p)); + } + return err; +} +EXPORT_SYMBOL(kernel_execve); ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-05-29 14:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-26 4:34 getxpid() parent lookup is broken Ben Hutchings 2012-05-29 4:55 ` Matt Turner 2012-05-29 14:09 ` Al Viro 2012-05-29 14:18 ` Al Viro
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).