cvs commit: src/sys/kern syscalls.master

[John Baldwin]

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 at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/