Date: Mon, 25 Apr 2005 10:41:55 +0200 From: des@des.no (=?iso-8859-1?q?Dag-Erling_Sm=F8rgrav?=) To: stable@freebsd.org Subject: pseudofs bugfixes Message-ID: <868y376yek.fsf@xps.des.no>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
Could somebody please test the attached patch on a recent RELENG_5?
It fixes a number of bugs in procfs and linprocfs; most notably, the
output of "ls /proc" (or "ls /compat/linux/proc") would be truncated
on systems with a largish number of processes (more than ~120).
There's a locking fix there too; please look out for LORs and let me
know if it introduces new ones (there are locking differences between
HEAD and RELENG_5, so sauce for the goose is not necessarily sauce for
the gander)
DES
--
Dag-Erling Smørgrav - des@des.no
[-- Attachment #2 --]
Index: sys/fs/pseudofs/pseudofs.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/pseudofs/pseudofs.c,v
retrieving revision 1.22
diff -u -r1.22 pseudofs.c
--- sys/fs/pseudofs/pseudofs.c 30 Jul 2004 22:08:50 -0000 1.22
+++ sys/fs/pseudofs/pseudofs.c 25 Apr 2005 08:35:19 -0000
@@ -24,10 +24,13 @@
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * $FreeBSD$
*/
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include "opt_pseudofs.h"
+
#include <sys/param.h>
#include <sys/kernel.h>
#include <sys/systm.h>
Index: sys/fs/pseudofs/pseudofs_fileno.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/pseudofs/pseudofs_fileno.c,v
retrieving revision 1.10
diff -u -r1.10 pseudofs_fileno.c
--- sys/fs/pseudofs/pseudofs_fileno.c 29 Apr 2003 13:36:01 -0000 1.10
+++ sys/fs/pseudofs/pseudofs_fileno.c 25 Apr 2005 08:35:19 -0000
@@ -24,10 +24,13 @@
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * $FreeBSD$
*/
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include "opt_pseudofs.h"
+
#include <sys/param.h>
#include <sys/kernel.h>
#include <sys/systm.h>
Index: sys/fs/pseudofs/pseudofs_vncache.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/pseudofs/pseudofs_vncache.c,v
retrieving revision 1.26
diff -u -r1.26 pseudofs_vncache.c
--- sys/fs/pseudofs/pseudofs_vncache.c 15 Aug 2004 21:58:02 -0000 1.26
+++ sys/fs/pseudofs/pseudofs_vncache.c 25 Apr 2005 08:35:19 -0000
@@ -24,10 +24,13 @@
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * $FreeBSD$
*/
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include "opt_pseudofs.h"
+
#include <sys/param.h>
#include <sys/kernel.h>
#include <sys/systm.h>
@@ -215,6 +218,10 @@
/*
* Free all vnodes associated with a defunct process
+ *
+ * XXXRW: It is unfortunate that pfs_exit() always acquires and releases two
+ * mutexes (one of which is Giant) for every process exit, even if procfs
+ * isn't mounted.
*/
static void
pfs_exit(void *arg, struct proc *p)
@@ -222,6 +229,8 @@
struct pfs_vdata *pvd;
struct vnode *vnp;
+ if (pfs_vncache == NULL)
+ return;
mtx_lock(&Giant);
/*
* This is extremely inefficient due to the fact that vgone() not
@@ -242,7 +251,9 @@
if (pvd->pvd_pid == p->p_pid) {
vnp = pvd->pvd_vnode;
mtx_unlock(&pfs_vncache_mutex);
+ VOP_LOCK(vnp, LK_EXCLUSIVE, curthread);
vgone(vnp);
+ VOP_UNLOCK(vnp, 0, curthread);
mtx_lock(&pfs_vncache_mutex);
pvd = pfs_vncache;
} else {
@@ -272,7 +283,9 @@
if (pvd->pvd_pn == pn) {
vnp = pvd->pvd_vnode;
mtx_unlock(&pfs_vncache_mutex);
+ VOP_LOCK(vnp, LK_EXCLUSIVE, curthread);
vgone(vnp);
+ VOP_UNLOCK(vnp, 0, curthread);
mtx_lock(&pfs_vncache_mutex);
pvd = pfs_vncache;
} else {
Index: sys/fs/pseudofs/pseudofs_vnops.c
===================================================================
RCS file: /home/ncvs/src/sys/fs/pseudofs/pseudofs_vnops.c,v
retrieving revision 1.45.2.1
diff -u -r1.45.2.1 pseudofs_vnops.c
--- sys/fs/pseudofs/pseudofs_vnops.c 6 Sep 2004 19:38:01 -0000 1.45.2.1
+++ sys/fs/pseudofs/pseudofs_vnops.c 25 Apr 2005 08:36:18 -0000
@@ -24,10 +24,13 @@
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * $FreeBSD$
*/
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include "opt_pseudofs.h"
+
#include <sys/param.h>
#include <sys/kernel.h>
#include <sys/systm.h>
@@ -36,6 +39,7 @@
#include <sys/fcntl.h>
#include <sys/limits.h>
#include <sys/lock.h>
+#include <sys/malloc.h>
#include <sys/mount.h>
#include <sys/mutex.h>
#include <sys/namei.h>
@@ -48,17 +52,25 @@
#include <fs/pseudofs/pseudofs.h>
#include <fs/pseudofs/pseudofs_internal.h>
-#if 0
+#ifdef PSEUDOFS_TRACE
+static int pfs_trace;
+SYSCTL_INT(_vfs_pfs, OID_AUTO, trace, CTLFLAG_RW, &pfs_trace, 0,
+ "enable tracing of pseudofs vnode operations");
+
#define PFS_TRACE(foo) \
do { \
- printf("pseudofs: %s(): line %d: ", __func__, __LINE__); \
- printf foo ; \
- printf("\n"); \
+ if (pfs_trace) { \
+ printf("%s(): line %d: ", __func__, __LINE__); \
+ printf foo ; \
+ printf("\n"); \
+ } \
} while (0)
#define PFS_RETURN(err) \
do { \
- printf("pseudofs: %s(): line %d: returning %d\n", \
- __func__, __LINE__, err); \
+ if (pfs_trace) { \
+ printf("%s(): line %d: returning %d\n", \
+ __func__, __LINE__, err); \
+ } \
return (err); \
} while (0)
#else
@@ -265,7 +277,7 @@
struct proc *proc = NULL;
int error;
- PFS_TRACE((pd->pn_name));
+ PFS_TRACE((pn->pn_name));
if (!pfs_visible(curthread, pn, pvd->pvd_pid))
PFS_RETURN (ENOENT);
@@ -299,18 +311,9 @@
/*
* Look up a file or directory
- *
- * XXX NOTE! pfs_lookup() has been hooked into vop_lookup_desc! This
- * will result in a lookup operation for a vnode which may already be
- * cached, therefore we have to be careful to purge the VFS cache when
- * reusing a vnode.
- *
- * This code will work, but is not really correct. Normally we would hook
- * vfs_cache_lookup() into vop_lookup_desc and hook pfs_lookup() into
- * vop_cachedlookup_desc.
*/
static int
-pfs_lookup(struct vop_lookup_args *va)
+pfs_lookup(struct vop_cachedlookup_args *va)
{
struct vnode *vn = va->a_dvp;
struct vnode **vpp = va->a_vpp;
@@ -319,24 +322,25 @@
struct pfs_node *pd = pvd->pvd_pn;
struct pfs_node *pn, *pdn = NULL;
pid_t pid = pvd->pvd_pid;
- int lockparent;
- int wantparent;
char *pname;
int error, i, namelen;
PFS_TRACE(("%.*s", (int)cnp->cn_namelen, cnp->cn_nameptr));
- cnp->cn_flags &= ~PDIRUNLOCK;
-
if (vn->v_type != VDIR)
PFS_RETURN (ENOTDIR);
+ error = VOP_ACCESS(vn, VEXEC, cnp->cn_cred, cnp->cn_thread);
+ if (error)
+ PFS_RETURN (error);
+
/*
* Don't support DELETE or RENAME. CREATE is supported so
* that O_CREAT will work, but the lookup will still fail if
* the file does not exist.
*/
- if (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME)
+ if ((cnp->cn_flags & ISLASTCN) &&
+ (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME))
PFS_RETURN (EOPNOTSUPP);
/* shortcut: check if the name is too long */
@@ -347,14 +351,10 @@
if (!pfs_visible(curthread, pd, pvd->pvd_pid))
PFS_RETURN (ENOENT);
- lockparent = cnp->cn_flags & LOCKPARENT;
- wantparent = cnp->cn_flags & (LOCKPARENT | WANTPARENT);
-
-
/* self */
namelen = cnp->cn_namelen;
pname = cnp->cn_nameptr;
- if (namelen == 1 && *pname == '.') {
+ if (namelen == 1 && pname[0] == '.') {
pn = pd;
*vpp = vn;
VREF(vn);
@@ -366,8 +366,6 @@
if (pd->pn_type == pfstype_root)
PFS_RETURN (EIO);
VOP_UNLOCK(vn, 0, cnp->cn_thread);
- cnp->cn_flags |= PDIRUNLOCK;
-
KASSERT(pd->pn_parent, ("non-root directory has no parent"));
/*
* This one is tricky. Descendents of procdir nodes
@@ -388,8 +386,8 @@
for (pn = pd->pn_nodes; pn != NULL; pn = pn->pn_next)
if (pn->pn_type == pfstype_procdir)
pdn = pn;
- else if (pn->pn_name[namelen] == '\0'
- && bcmp(pname, pn->pn_name, namelen) == 0)
+ else if (pn->pn_name[namelen] == '\0' &&
+ bcmp(pname, pn->pn_name, namelen) == 0)
goto got_pnode;
/* process dependent node */
@@ -406,28 +404,24 @@
got_pnode:
if (pn != pd->pn_parent && !pn->pn_parent)
pn->pn_parent = pd;
- if (!pfs_visible(curthread, pn, pvd->pvd_pid))
- PFS_RETURN (ENOENT);
+ if (!pfs_visible(curthread, pn, pvd->pvd_pid)) {
+ error = ENOENT;
+ goto failed;
+ }
error = pfs_vncache_alloc(vn->v_mount, vpp, pn, pid);
if (error)
- PFS_RETURN (error);
+ goto failed;
- if ((cnp->cn_flags & ISDOTDOT) && (cnp->cn_flags & ISLASTCN)
- && lockparent) {
+ if (cnp->cn_flags & ISDOTDOT)
vn_lock(vn, LK_EXCLUSIVE|LK_RETRY, cnp->cn_thread);
- cnp->cn_flags &= ~PDIRUNLOCK;
- }
- if (!((lockparent && (cnp->cn_flags & ISLASTCN)) ||
- (cnp->cn_flags & ISDOTDOT)))
- VOP_UNLOCK(vn, 0, cnp->cn_thread);
-
- /*
- * XXX See comment at top of the routine.
- */
if (cnp->cn_flags & MAKEENTRY)
cache_enter(vn, *vpp, cnp);
PFS_RETURN (0);
+ failed:
+ if (cnp->cn_flags & ISDOTDOT)
+ vn_lock(vn, LK_EXCLUSIVE|LK_RETRY, cnp->cn_thread);
+ PFS_RETURN(error);
}
/*
@@ -606,7 +600,7 @@
struct proc *p;
off_t offset;
int error, i, resid;
- struct sbuf sb;
+ char *buf, *ent;
PFS_TRACE((pd->pn_name));
@@ -621,11 +615,11 @@
/* only allow reading entire entries */
offset = uio->uio_offset;
resid = uio->uio_resid;
- if (offset < 0 || offset % PFS_DELEN != 0 || resid < PFS_DELEN)
+ if (offset < 0 || offset % PFS_DELEN != 0 ||
+ (resid && resid < PFS_DELEN))
PFS_RETURN (EINVAL);
-
- if (sbuf_new(&sb, NULL, resid, SBUF_FIXEDLEN) == NULL)
- PFS_RETURN (ENOMEM);
+ if (resid == 0)
+ PFS_RETURN (0);
/* skip unwanted entries */
sx_slock(&allproc_lock);
@@ -633,13 +627,14 @@
if (pfs_iterate(curthread, pid, pd, &pn, &p) == -1) {
/* nothing left... */
sx_sunlock(&allproc_lock);
- sbuf_delete(&sb);
PFS_RETURN (0);
}
/* fill in entries */
+ ent = buf = malloc(resid, M_IOV, M_WAITOK);
entry.d_reclen = PFS_DELEN;
- while (pfs_iterate(curthread, pid, pd, &pn, &p) != -1 && resid > 0) {
+ while (pfs_iterate(curthread, pid, pd, &pn, &p) != -1 &&
+ resid >= PFS_DELEN) {
if (!pn->pn_parent)
pn->pn_parent = pd;
if (!pn->pn_fileno)
@@ -677,14 +672,14 @@
panic("%s has unexpected node type: %d", pn->pn_name, pn->pn_type);
}
PFS_TRACE((entry.d_name));
- sbuf_bcat(&sb, &entry, PFS_DELEN);
+ bcopy(&entry, ent, PFS_DELEN); /* XXX waste of cycles */
offset += PFS_DELEN;
resid -= PFS_DELEN;
+ ent += PFS_DELEN;
}
sx_sunlock(&allproc_lock);
- sbuf_finish(&sb);
- error = uiomove(sbuf_data(&sb), sbuf_len(&sb), uio);
- sbuf_delete(&sb);
+ error = uiomove(buf, ent - buf, uio);
+ free(buf, M_IOV);
PFS_RETURN (error);
}
@@ -763,7 +758,7 @@
* Read from a file
*/
static int
-pfs_write(struct vop_read_args *va)
+pfs_write(struct vop_write_args *va)
{
struct vnode *vn = va->a_vp;
struct pfs_vdata *pvd = (struct pfs_vdata *)vn->v_data;
@@ -826,13 +821,14 @@
static struct vnodeopv_entry_desc pfs_vnodeop_entries[] = {
{ &vop_default_desc, (vop_t *)vop_defaultop },
{ &vop_access_desc, (vop_t *)pfs_access },
+ { &vop_cachedlookup_desc, (vop_t *)pfs_lookup },
{ &vop_close_desc, (vop_t *)pfs_close },
{ &vop_create_desc, (vop_t *)vop_eopnotsupp },
{ &vop_getattr_desc, (vop_t *)pfs_getattr },
{ &vop_getextattr_desc, (vop_t *)pfs_getextattr },
{ &vop_ioctl_desc, (vop_t *)pfs_ioctl },
{ &vop_link_desc, (vop_t *)vop_eopnotsupp },
- { &vop_lookup_desc, (vop_t *)pfs_lookup },
+ { &vop_lookup_desc, (vop_t *)vfs_cache_lookup },
{ &vop_mkdir_desc, (vop_t *)vop_eopnotsupp },
{ &vop_mknod_desc, (vop_t *)vop_eopnotsupp },
{ &vop_open_desc, (vop_t *)pfs_open },
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?868y376yek.fsf>