Date: Fri, 14 Nov 2003 11:29:33 -0500 (EST) From: John Baldwin <jhb@FreeBSD.org> To: Jeff Roberson <jeff@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: RE: cvs commit: src/sys/kern syscalls.master Message-ID: <XFMail.20031114112933.jhb@FreeBSD.org> In-Reply-To: <200311140348.hAE3mbRt083137@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 14-Nov-2003 Jeff Roberson wrote:
> jeff 2003/11/13 19:48:37 PST
>
> FreeBSD src repository
>
> Modified files:
> sys/kern syscalls.master
> Log:
> - Revision 1.156 marked ptrace() SMP safe. Unfortunately, alpha implements
> parts of ptrace using proc_rwmem(). proc_rwmem() requires giant, and
> giant must be acquired prior to the proc lock, so ptrace must require giant
> still.
case PT_READ_I:
case PT_READ_D:
PROC_UNLOCK(p);
tmp = 0;
/* write = 0 set above */
iov.iov_base = write ? (caddr_t)&data : (caddr_t)&tmp;
iov.iov_len = sizeof(int);
uio.uio_iov = &iov;
uio.uio_iovcnt = 1;
uio.uio_offset = (off_t)(uintptr_t)addr;
uio.uio_resid = sizeof(int);
uio.uio_segflg = UIO_SYSSPACE; /* i.e.: the uap */
uio.uio_rw = write ? UIO_WRITE : UIO_READ;
uio.uio_td = td;
mtx_lock(&Giant);
error = proc_rwmem(p, &uio);
mtx_unlock(&Giant);
if (uio.uio_resid != 0) {
/*
* XXX proc_rwmem() doesn't currently return ENOSPC,
* so I think write() can bogusly return 0.
* XXX what happens for short writes? We don't want
* to write partial data.
* XXX proc_rwmem() returns EPERM for other invalid
* addresses. Convert this to EINVAL. Does this
* clobber returns of EPERM for other reasons?
*/
if (error == 0 || error == ENOSPC || error == EPERM)
error = EINVAL; /* EOF */
}
if (!write)
td->td_retval[0] = tmp;
return (error);
Are there other parts on Alpha that use proc_rwmem()? Ah. It should be
fine to drop the proc lock in some places then I think. For example,
around ptrace_single_step(). Something like:
Index: sys_process.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/sys_process.c,v
retrieving revision 1.115
diff -u -r1.115 sys_process.c
--- sys_process.c 9 Oct 2003 10:17:16 -0000 1.115
+++ sys_process.c 14 Nov 2003 15:47:46 -0000
@@ -522,11 +522,13 @@
switch (req) {
case PT_STEP:
+ PROC_UNLOCK(p);
error = ptrace_single_step(td2);
if (error) {
- _PRELE(p);
- goto fail;
+ PRELE(p);
+ goto fail_noproc;
}
+ PROC_LOCK(p);
break;
case PT_TO_SCE:
p->p_stops |= S_PT_SCE;
@@ -540,11 +542,13 @@
}
if (addr != (void *)1) {
+ PROC_UNLOCK(p);
error = ptrace_set_pc(td2, (u_long)(uintfptr_t)addr);
if (error) {
- _PRELE(p);
- goto fail;
+ PRELE(p);
+ goto fail_noproc;
}
+ PROC_LOCK(p);
}
_PRELE(p);
@@ -703,9 +707,9 @@
#ifdef __HAVE_PTRACE_MACHDEP
if (req >= PT_FIRSTMACH) {
_PHOLD(p);
- error = cpu_ptrace(td2, req, addr, data);
- _PRELE(p);
PROC_UNLOCK(p);
+ error = cpu_ptrace(td2, req, addr, data);
+ PRELE(p);
return (error);
}
#endif
@@ -717,6 +721,7 @@
fail:
PROC_UNLOCK(p);
+fail_noproc:
if (proctree_locked)
sx_xunlock(&proctree_lock);
return (error);
We can do the same around the read/write registers stuff as well given
that they use PHOLD/PRELE if need be.
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20031114112933.jhb>