Date: Wed, 11 Jul 2007 03:20:09 GMT From: Jan Harkes <jaharkes@cs.cmu.edu> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/95891: [coda] [panic] kernel panic when coda6_client Message-ID: <200707110320.l6B3K9pP088302@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/95891; it has been noted by GNATS.
From: Jan Harkes <jaharkes@cs.cmu.edu>
To: bug-followup@FreeBSD.org
Cc:
Subject: Re: kern/95891: [coda] [panic] kernel panic when coda6_client
Date: Tue, 10 Jul 2007 22:33:32 -0400
--WIyZ46R2i8wDzkSu
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Attached is a patch series to fix various panics and other issues when
trying to use the Coda kernel module. I hope GNATS can handle multipart
attachments.
To actually be able to run a Coda client, you need a more recent version
of the userspace components which use the new nmount sycall.
lwp-2.3
rpc2-2.5
rvm-1.14
coda-6.9.1
These can be downloaded from,
ftp://ftp.coda.cs.cmu.edu/pub/coda/src/
Jan
--WIyZ46R2i8wDzkSu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0001-Avoid-crash-when-opening-Coda-device.patch"
From d459d345c7b3e3c9deaa5cf5834cb25f53d007f2 Mon Sep 17 00:00:00 2001
From: Jan Harkes <jaharkes@cs.cmu.edu>
Date: Tue, 7 Nov 2006 10:20:46 -0500
Subject: [PATCH Coda 1/5] Avoid crash when opening Coda device
When allocating coda_mntinfo, we need to initialize dev so that we
can actually find the allocated coda_mntinfo structure later on.
---
coda_fbsd.c | 5 +++--
coda_psdev.c | 12 +++++++-----
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/coda_fbsd.c b/coda_fbsd.c
index 8a74ada..5a603de 100644
--- a/coda_fbsd.c
+++ b/coda_fbsd.c
@@ -124,6 +124,7 @@ static void coda_fbsd_clone(arg, cred, name, namelen, dev)
dev_ref(*dev);
mnt = malloc(sizeof(struct coda_mntinfo), M_CODA, M_WAITOK|M_ZERO);
LIST_INSERT_HEAD(&coda_mnttbl, mnt, mi_list);
+ mnt->dev = *dev;
}
struct coda_mntinfo *
@@ -133,8 +134,8 @@ dev2coda_mntinfo(struct cdev *dev)
LIST_FOREACH(mnt, &coda_mnttbl, mi_list) {
if (mnt->dev == dev)
- break;
+ return mnt;
}
- return mnt;
+ return NULL;
}
diff --git a/coda_psdev.c b/coda_psdev.c
index 7d35540..d5d965a 100644
--- a/coda_psdev.c
+++ b/coda_psdev.c
@@ -129,6 +129,8 @@ vc_nb_open(dev, flag, mode, td)
coda_nc_init();
mnt = dev2coda_mntinfo(dev);
+ KASSERT(mnt, ("Coda: tried to open uninitialized cfs device"));
+
vcp = &mnt->mi_vcomm;
if (VC_OPEN(vcp))
return(EBUSY);
@@ -154,15 +156,15 @@ vc_nb_close (dev, flag, mode, td)
register struct vcomm *vcp;
register struct vmsg *vmp, *nvmp = NULL;
struct coda_mntinfo *mi;
- int err;
+ int err;
ENTRY;
mi = dev2coda_mntinfo(dev);
- vcp = &(mi->mi_vcomm);
-
- if (!VC_OPEN(vcp))
- panic("vcclose: not open");
+ KASSERT(mi, ("Coda: closing unknown cfs device"));
+
+ vcp = &mi->mi_vcomm;
+ KASSERT(VC_OPEN(vcp), ("Coda: closing unopened cfs device"));
/* prevent future operations on this vfs from succeeding by auto-
* unmounting any vfs mounted via this device. This frees user or
--
1.5.2.1
--WIyZ46R2i8wDzkSu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0002-Mount-was-failing-because-we-failed-to-match-the-dev.patch"
From 52ef0113ff3d8517546b2235ad98746ea8eb96bf Mon Sep 17 00:00:00 2001
From: Jan Harkes <jaharkes@cs.cmu.edu>
Date: Tue, 7 Nov 2006 10:22:07 -0500
Subject: [PATCH Coda 2/5] Mount was failing because we failed to match the device operations.
But we don't have to, if we find the coda_mntinfo structure for this
device in our linked list, we know the device is good.
---
coda_vfsops.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/coda_vfsops.c b/coda_vfsops.c
index 44538a2..68fdafe 100644
--- a/coda_vfsops.c
+++ b/coda_vfsops.c
@@ -153,19 +153,15 @@ coda_mount(struct mount *vfsp, struct thread *td)
NDFREE(&ndp, NDF_ONLY_PNBUF);
/*
- * See if the device table matches our expectations.
+ * Initialize the mount record and link it to the vfs struct
*/
- if (dev->si_devsw->d_open != vc_nb_open)
- {
+ mi = dev2coda_mntinfo(dev);
+ if (!mi) {
MARK_INT_FAIL(CODA_MOUNT_STATS);
+ printf("Coda mount: %s is not a cfs device\n", from);
return(ENXIO);
}
- /*
- * Initialize the mount record and link it to the vfs struct
- */
- mi = dev2coda_mntinfo(dev);
-
if (!VC_OPEN(&mi->mi_vcomm)) {
MARK_INT_FAIL(CODA_MOUNT_STATS);
return(ENODEV);
--
1.5.2.1
--WIyZ46R2i8wDzkSu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0003-Replace-CODA_OPEN-with-CODA_OPEN_BY_FD.patch"
From 56e4feceb1f89c4460a9821dc822f152b609e559 Mon Sep 17 00:00:00 2001
From: Jan Harkes <jaharkes@cs.cmu.edu>
Date: Wed, 8 Nov 2006 09:38:41 -0500
Subject: [PATCH Coda 3/5] Replace CODA_OPEN with CODA_OPEN_BY_FD
coda_open was disabled because we can't open container files by
device/inode number pair anymore. Replace the CODA_OPEN upcall
with CODA_OPEN_BY_FD, where venus returns an open file descriptor
for the container file. We can then grab a reference on the vnode
coda_psdev.c:vc_nb_write and use this vnode for further accesses
to the container file.
---
cnode.h | 2 -
coda.h | 2 +-
coda_psdev.c | 23 +++++++++-
coda_venus.c | 19 +++------
coda_venus.h | 2 +-
coda_vnops.c | 137 ++++++++++-----------------------------------------------
6 files changed, 54 insertions(+), 131 deletions(-)
diff --git a/cnode.h b/cnode.h
index 0ab2ab9..26cbaaa 100644
--- a/cnode.h
+++ b/cnode.h
@@ -107,8 +107,6 @@ struct cnode {
struct vattr c_vattr; /* attributes */
char *c_symlink; /* pointer to symbolic link */
u_short c_symlen; /* length of symbolic link */
- struct cdev *c_device; /* associated vnode device */
- ino_t c_inode; /* associated vnode inode */
struct cnode *c_next; /* links if on NetBSD machine */
};
#define VTOC(vp) ((struct cnode *)(vp)->v_data)
diff --git a/coda.h b/coda.h
index 13cae28..5bbc193 100644
--- a/coda.h
+++ b/coda.h
@@ -679,7 +679,7 @@ struct coda_open_by_fd_in {
struct coda_open_by_fd_out {
struct coda_out_hdr oh;
int fd;
- struct file *fh;
+ struct vnode *vp;
};
/* coda_open_by_path: */
diff --git a/coda_psdev.c b/coda_psdev.c
index d5d965a..82307c0 100644
--- a/coda_psdev.c
+++ b/coda_psdev.c
@@ -65,6 +65,7 @@ extern int coda_nc_initialized; /* Set if cache has been initialized */
#include <sys/mutex.h>
#include <sys/poll.h>
#include <sys/proc.h>
+#include <sys/filedesc.h>
#include <coda/coda.h>
#include <coda/cnode.h>
@@ -370,9 +371,29 @@ vc_nb_write(dev, uiop, flag)
out->unique = seq;
vmp->vm_outSize = buf[0]; /* Amount of data transferred? */
vmp->vm_flags |= VM_WRITE;
+
+ error = 0;
+ if (opcode == CODA_OPEN_BY_FD) {
+ struct coda_open_by_fd_out *tmp = (struct coda_open_by_fd_out *)out;
+ struct file *fp;
+ struct vnode *vp = NULL;
+
+ if (tmp->oh.result == 0) {
+ error = getvnode(uiop->uio_td->td_proc->p_fd, tmp->fd, &fp);
+ if (!error) {
+ mtx_lock(&Giant);
+ vp = fp->f_vnode;
+ VREF(vp);
+ fdrop(fp, uiop->uio_td);
+ mtx_unlock(&Giant);
+ }
+ }
+ tmp->vp = vp;
+ }
+
wakeup(&vmp->vm_sleep);
- return(0);
+ return(error);
}
int
diff --git a/coda_venus.c b/coda_venus.c
index cce9f4a..5d7ad2b 100644
--- a/coda_venus.c
+++ b/coda_venus.c
@@ -198,30 +198,23 @@ venus_root(void *mdp,
int
venus_open(void *mdp, CodaFid *fid, int flag,
struct ucred *cred, struct proc *p,
-/*out*/ struct cdev **dev, ino_t *inode)
+/*out*/ struct vnode **vp)
{
-#if 0
int cflag;
- DECL(coda_open); /* sets Isize & Osize */
- ALLOC(coda_open); /* sets inp & outp */
+ DECL(coda_open_by_fd); /* sets Isize & Osize */
+ ALLOC(coda_open_by_fd); /* sets inp & outp */
/* send the open to venus. */
- INIT_IN(&inp->ih, CODA_OPEN, cred, p);
+ INIT_IN(&inp->ih, CODA_OPEN_BY_FD, cred, p);
inp->Fid = *fid;
CNV_OFLAG(cflag, flag);
inp->flags = cflag;
error = coda_call(mdp, Isize, &Osize, (char *)inp);
- if (!error) {
- *dev = findcdev(outp->dev);
- *inode = outp->inode;
- }
+ *vp = error ? NULL : outp->vp;
- CODA_FREE(inp, coda_open_size);
+ CODA_FREE(inp, coda_open_by_fd_size);
return error;
-#else
- return (EOPNOTSUPP);
-#endif
}
int
diff --git a/coda_venus.h b/coda_venus.h
index 329ea32..8f0acb6 100644
--- a/coda_venus.h
+++ b/coda_venus.h
@@ -39,7 +39,7 @@ venus_root(void *mdp,
int
venus_open(void *mdp, CodaFid *fid, int flag,
struct ucred *cred, struct proc *p,
-/*out*/ struct cdev **dev, ino_t *inode);
+/*out*/ struct vnode **vp);
int
venus_close(void *mdp, CodaFid *fid, int flag,
diff --git a/coda_vnops.c b/coda_vnops.c
index 7d06764..c75fbed 100644
--- a/coda_vnops.c
+++ b/coda_vnops.c
@@ -178,9 +178,10 @@ coda_vnodeopstats_init(void)
}
/*
- * coda_open calls Venus to return the device, inode pair of the cache
- * file holding the data. Using iget, coda_open finds the vnode of the
- * cache file, and then opens it.
+ * coda_open calls Venus which returns an open file descriptor the cache
+ * file holding the data. We get the vnode while we are still in the
+ * context of the venus process in coda_psdev.c. This vnode is then
+ * passed back to the caller and opened.
*/
int
coda_open(struct vop_open_args *ap)
@@ -199,8 +200,6 @@ coda_open(struct vop_open_args *ap)
/* locals */
int error;
struct vnode *vp;
- struct cdev *dev;
- ino_t inode;
MARK_ENTRY(CODA_OPEN_STATS);
@@ -216,23 +215,12 @@ coda_open(struct vop_open_args *ap)
return(0);
}
- error = venus_open(vtomi((*vpp)), &cp->c_fid, flag, cred, td->td_proc, &dev, &inode);
+ error = venus_open(vtomi((*vpp)), &cp->c_fid, flag, cred, td->td_proc, &vp);
if (error)
return (error);
- if (!error) {
- CODADEBUG( CODA_OPEN,myprintf(("open: dev %#lx inode %lu result %d\n",
- (u_long)dev2udev(dev), (u_long)inode,
- error)); )
- }
- /* Translate the <device, inode> pair for the cache file into
- an inode pointer. */
- error = coda_grab_vnode(dev, inode, &vp);
- if (error)
- return (error);
+ CODADEBUG( CODA_OPEN,myprintf(("open: vp %p result %d\n", vp, error));)
- /* We get the vnode back locked. Needs unlocked */
- VOP_UNLOCK(vp, 0, td);
/* Keep a reference until the close comes in. */
vref(*vpp);
@@ -251,11 +239,6 @@ coda_open(struct vop_open_args *ap)
cp->c_flags &= ~C_VATTR;
}
- /* Save the <device, inode> pair for the cache file to speed
- up subsequent page_read's. */
- cp->c_device = dev;
- cp->c_inode = inode;
-
/* Open the cache file. */
error = VOP_OPEN(vp, flag, cred, td, NULL);
if (error) {
@@ -290,28 +273,13 @@ coda_close(struct vop_close_args *ap)
return(0);
}
- if (IS_UNMOUNTING(cp)) {
- if (cp->c_ovp) {
-#ifdef CODA_VERBOSE
- printf("coda_close: destroying container ref %d, ufs vp %p of vp %p/cp %p\n",
- vrefcnt(vp), cp->c_ovp, vp, cp);
-#endif
-#ifdef hmm
- vgone(cp->c_ovp);
-#else
- VOP_CLOSE(cp->c_ovp, flag, cred, td); /* Do errors matter here? */
- vrele(cp->c_ovp);
-#endif
- } else {
-#ifdef CODA_VERBOSE
- printf("coda_close: NO container vp %p/cp %p\n", vp, cp);
-#endif
- }
- return ENODEV;
- } else {
+ if (cp->c_ovp) {
VOP_CLOSE(cp->c_ovp, flag, cred, td); /* Do errors matter here? */
vrele(cp->c_ovp);
}
+#ifdef CODA_VERBOSE
+ else printf("coda_close: NO container vp %p/cp %p\n", vp, cp);
+#endif
if (--cp->c_ocount == 0)
cp->c_ovp = NULL;
@@ -319,8 +287,11 @@ coda_close(struct vop_close_args *ap)
if (flag & FWRITE) /* file was opened for write */
--cp->c_owrite;
- error = venus_close(vtomi(vp), &cp->c_fid, flag, cred, td->td_proc);
- vrele(CTOV(cp));
+ if (!IS_UNMOUNTING(cp))
+ error = venus_close(vtomi(vp), &cp->c_fid, flag, cred, td->td_proc);
+ else error = ENODEV;
+
+ vrele(vp);
CODADEBUG(CODA_CLOSE, myprintf(("close: result %d\n",error)); )
return(error);
@@ -353,12 +324,8 @@ coda_rdwr(struct vnode *vp, struct uio *uiop, enum uio_rw rw, int ioflag,
/* locals */
struct cnode *cp = VTOC(vp);
struct vnode *cfvp = cp->c_ovp;
- struct proc *p = td->td_proc;
- struct thread *ltd = td;
- int igot_internally = 0;
int opened_internally = 0;
int error = 0;
- int iscore = 0;
MARK_ENTRY(CODA_RDWR_STATS);
@@ -373,51 +340,19 @@ coda_rdwr(struct vnode *vp, struct uio *uiop, enum uio_rw rw, int ioflag,
}
/*
- * If file is not already open this must be a page
- * {read,write} request. Iget the cache file's inode
- * pointer if we still have its <device, inode> pair.
- * Otherwise, we must do an internal open to derive the
- * pair.
+ * If file is not already open this must be a page {read,write} request
+ * and we should open it internally.
*/
if (cfvp == NULL) {
- /*
- * If we're dumping core, do the internal open. Otherwise
- * venus won't have the correct size of the core when
- * it's completely written.
- */
- if (p) {
- PROC_LOCK(p);
- iscore = (p->p_acflag & ACORE);
- PROC_UNLOCK(p);
- }
- else
- ltd = curthread;
-
- if (cp->c_inode != 0 && !iscore) {
- igot_internally = 1;
- error = coda_grab_vnode(cp->c_device, cp->c_inode, &cfvp);
- if (error) {
- MARK_INT_FAIL(CODA_RDWR_STATS);
- return(error);
- }
- /*
- * We get the vnode back locked by curthread in both Mach and
- * NetBSD. Needs unlocked
- */
- VOP_UNLOCK(cfvp, 0, ltd);
- }
- else {
- opened_internally = 1;
- MARK_INT_GEN(CODA_OPEN_STATS);
- error = VOP_OPEN(vp, (rw == UIO_READ ? FREAD : FWRITE),
- cred, td, NULL);
-printf("coda_rdwr: Internally Opening %p\n", vp);
- if (error) {
+ opened_internally = 1;
+ MARK_INT_GEN(CODA_OPEN_STATS);
+ error = VOP_OPEN(vp, (rw == UIO_READ ? FREAD : FWRITE), cred, td, NULL);
+ printf("coda_rdwr: Internally Opening %p\n", vp);
+ if (error) {
printf("coda_rdwr: VOP_OPEN on container failed %d\n", error);
return (error);
- }
- cfvp = cp->c_ovp;
}
+ cfvp = cp->c_ovp;
}
/* Have UFS handle the call. */
@@ -1526,7 +1461,7 @@ coda_readdir(struct vop_readdir_args *ap)
opened_internally = 1;
MARK_INT_GEN(CODA_OPEN_STATS);
error = VOP_OPEN(vp, FREAD, cred, td, NULL);
-printf("coda_readdir: Internally Opening %p\n", vp);
+ printf("coda_readdir: Internally Opening %p\n", vp);
if (error) {
printf("coda_readdir: VOP_OPEN on container failed %d\n", error);
return (error);
@@ -1677,30 +1612,6 @@ coda_islocked(struct vop_islocked_args *ap)
return (vop_stdislocked(ap));
}
-/* How one looks up a vnode given a device/inode pair: */
-int
-coda_grab_vnode(struct cdev *dev, ino_t ino, struct vnode **vpp)
-{
- /* This is like VFS_VGET() or igetinode()! */
- int error;
- struct mount *mp;
-
- if (!(mp = devtomp(dev))) {
- myprintf(("coda_grab_vnode: devtomp(%#lx) returns NULL\n",
- (u_long)dev2udev(dev)));
- return(ENXIO);
- }
-
- /* XXX - ensure that nonzero-return means failure */
- error = VFS_VGET(mp,ino,LK_EXCLUSIVE,vpp);
- if (error) {
- myprintf(("coda_grab_vnode: iget/vget(%lx, %lu) returns %p, err %d\n",
- (u_long)dev2udev(dev), (u_long)ino, (void *)*vpp, error));
- return(ENOENT);
- }
- return(0);
-}
-
void
print_vattr(struct vattr *attr)
{
--
1.5.2.1
--WIyZ46R2i8wDzkSu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0004-Avoid-a-panic-in-insmntque-when-we-pass-a-NULL-mount.patch"
From aabb34fa9abb18011522d7bf2881c4df9ca4d325 Mon Sep 17 00:00:00 2001
From: Jan Harkes <jaharkes@cs.cmu.edu>
Date: Sat, 12 May 2007 23:38:35 -0400
Subject: [PATCH Coda 4/5] Avoid a panic in insmntque when we pass a NULL mount
This reenables some previously disabled code which according to the
comment caused a problem during shutdown. But even that is still
better than triggering a kernel panic whenever venus is started.
---
coda_vfsops.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/coda_vfsops.c b/coda_vfsops.c
index 68fdafe..07d0ad1 100644
--- a/coda_vfsops.c
+++ b/coda_vfsops.c
@@ -183,13 +183,7 @@ coda_mount(struct mount *vfsp, struct thread *td)
rootvp = CTOV(cp);
rootvp->v_vflag |= VV_ROOT;
-/* cp = make_coda_node(&ctlfid, vfsp, VCHR);
- The above code seems to cause a loop in the cnode links.
- I don't totally understand when it happens, it is caught
- when closing down the system.
- */
- cp = make_coda_node(&ctlfid, 0, VCHR);
-
+ cp = make_coda_node(&ctlfid, vfsp, VCHR);
coda_ctlvp = CTOV(cp);
/* Add vfs and rootvp to chain of vfs hanging off mntinfo */
--
1.5.2.1
--WIyZ46R2i8wDzkSu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0005-Fix-ioctls-on-the-control-vnode.patch"
From 901d097d613bcfad295db9e9b2a460c76f11690f Mon Sep 17 00:00:00 2001
From: Jan Harkes <jaharkes@cs.cmu.edu>
Date: Sun, 13 May 2007 18:49:49 -0400
Subject: [PATCH Coda 5/5] Fix ioctls on the control vnode
Ioctls on a character device fail with ENOTTY. Make the control vnode
a regular file so that ioctls are passed through to our kernel module.
---
coda_vfsops.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/coda_vfsops.c b/coda_vfsops.c
index 07d0ad1..1f8b9da 100644
--- a/coda_vfsops.c
+++ b/coda_vfsops.c
@@ -183,7 +183,7 @@ coda_mount(struct mount *vfsp, struct thread *td)
rootvp = CTOV(cp);
rootvp->v_vflag |= VV_ROOT;
- cp = make_coda_node(&ctlfid, vfsp, VCHR);
+ cp = make_coda_node(&ctlfid, vfsp, VREG);
coda_ctlvp = CTOV(cp);
/* Add vfs and rootvp to chain of vfs hanging off mntinfo */
--
1.5.2.1
--WIyZ46R2i8wDzkSu--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200707110320.l6B3K9pP088302>