Date: Tue, 9 Jul 2002 08:58:38 -0700 (PDT) From: Don Lewis <dl-freebsd@catspoiler.org> To: jroberson@chesapeake.net, current@FreeBSD.ORG Cc: jroberson@chesapeake.net, current@FreeBSD.ORG Subject: code ordering in coredump() (was: Re: cvs commit: src/sys/tools vnode_if.awk) Message-ID: <200207091558.g69Fwcwr004219@gw.catspoiler.org> In-Reply-To: <200207081423.g68ENiwr000522@gw.catspoiler.org>
next in thread | previous in thread | raw e-mail | index | archive | help
I was studying the following DEBUG_VFS_LOCKS panic and noticed something
bothersome about the ordering of the code in coredump(). It looked to
me like it made more sense to verify that the file was something that
was valid to dump to before doing the vn_start_write() stuff.
Rearranging the code also allows VOP_GETATTR() to be called before
dropping the initial lock.
> VOP_GETATTR: 0xc7445100 is not locked but should be
> Debugger("Lock violation.
> ")
> Stopped at Debugger+0x45: xchgl %ebx,in_Debugger.0
> db> tr
> Debugger(c041b364) at Debugger+0x45
> coredump(c6a2c180) at coredump+0x470
> sigexit(c6a2c180,b,0,e54f8000,c6a2c180) at sigexit+0xa4
> postsig(b) at postsig+0xe9
> ast(e5514d48) at ast+0x27e
> doreti_ast() at doreti_ast+0x1a
Index: kern_sig.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.174
diff -u -r1.174 kern_sig.c
--- kern_sig.c 3 Jul 2002 09:15:20 -0000 1.174
+++ kern_sig.c 9 Jul 2002 15:17:31 -0000
@@ -1967,8 +1967,6 @@
* then it passes on a vnode and a size limit to the process-specific
* coredump routine if there is one; if there _is not_ one, it returns
* ENOSYS; otherwise it returns the error from the process-specific routine.
- *
- * XXX: VOP_GETATTR() here requires holding the vnode lock.
*/
static int
@@ -2021,6 +2019,14 @@
NDFREE(&nd, NDF_ONLY_PNBUF);
vp = nd.ni_vp;
+ /* Don't dump to non-regular files or files with links. */
+ if (vp->v_type != VREG ||
+ VOP_GETATTR(vp, &vattr, cred, td) || vattr.va_nlink != 1) {
+ VOP_UNLOCK(vp, 0, td);
+ error = EFAULT;
+ goto out2;
+ }
+
VOP_UNLOCK(vp, 0, td);
lf.l_whence = SEEK_SET;
lf.l_start = 0;
@@ -2040,12 +2046,6 @@
goto restart;
}
- /* Don't dump to non-regular files or files with links. */
- if (vp->v_type != VREG ||
- VOP_GETATTR(vp, &vattr, cred, td) || vattr.va_nlink != 1) {
- error = EFAULT;
- goto out1;
- }
VATTR_NULL(&vattr);
vattr.va_size = 0;
vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
@@ -2060,7 +2060,6 @@
p->p_sysent->sv_coredump(td, vp, limit) :
ENOSYS;
-out1:
lf.l_type = F_UNLCK;
VOP_ADVLOCK(vp, (caddr_t)p, F_UNLCK, &lf, F_FLOCK);
vn_finished_write(mp);
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200207091558.g69Fwcwr004219>