[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#718888: Fwd: nullfs page fault (triggered by FAM)



On 08/08/2013 13:46, Robert Millan wrote:
Package: kfreebsd-image-9.0-2-amd64
Version: 9.0-10+deb70.1
Severity: grave

A few days ago I started getting kernel page faults. In my setup the
problem is 100% reproducible and triggered by the following conditions:

- FreeBSD chroot with nullfs mounts for /home and /tmp.

- Run thunderbird within the chroot. Within a minute, kernel page faults.

A backtrace is attached, which points to null_remove as the culprit.
Also, the process triggering this is part of FAM (File Alteration
Monitor) framework, which makes me suspect the kind of agressive
filesystem usage of this library is related to the panic.

As the problem is not reproducible with upstream kernel, I tried
disabling our nullfs-related patches. It turns out removing
101_nullfs_vsock.diff makes the problem disappear!

Also, much to my surprise, the problem 101_nullfs_vsock.diff is supposed
to fix (unavailability of X service to apps within the chroot) doesn't
manifest itself. Could it be that upstream had already fixed this in
another way by the time 9.0 was released?

Some advice would be appreciated. I'm inclined to remove the patch if
possible.
Perhaps a more conservative approach, could be to replace the patch with the one that went into 9-STABLE (attached).

What do you think? If noone objects, I'd like to request pre-approval to -release to include it in Wheezy.

-- 
Robert Millan
Index: UPDATING
===================================================================
--- UPDATING	(revision 234659)
+++ UPDATING	(revision 234660)
@@ -9,6 +9,14 @@
 Items affecting the ports and packages system can be found in
 /usr/ports/UPDATING.  Please read that file before running portupgrade.
 
+20120422:
+	Now unix domain sockets behave "as expected" on	nullfs(5). Previously
+	nullfs(5) did not pass through all behaviours to the underlying layer,
+	as a result if we bound to a socket on the lower layer we could connect
+	only to the lower path; if we bound to the upper layer we could connect
+	only to	the upper path. The new behavior is one can connect to both the
+	lower and the upper paths regardless what layer path one binds to.
+
 20120109:
 	The acpi_wmi(4) status device /dev/wmistat has been renamed to
 	/dev/wmistat0.

Property changes on: UPDATING
___________________________________________________________________
Modified: svn:mergeinfo
   Merged /head/UPDATING:r232317

Index: sys/kern/vnode_if.src
===================================================================
--- sys/kern/vnode_if.src	(revision 234659)
+++ sys/kern/vnode_if.src	(revision 234660)
@@ -640,23 +640,31 @@
 	IN int advice;
 };
 
-# The VOPs below are spares at the end of the table to allow new VOPs to be
-# added in stable branches without breaking the KBI.  New VOPs in HEAD should
-# be added above these spares.  When merging a new VOP to a stable branch,
-# the new VOP should replace one of the spares.
+%% unp_bind	vp	E E E
 
-vop_spare1 {
+vop_unp_bind {
 	IN struct vnode *vp;
+	IN struct socket *socket;
 };
 
-vop_spare2 {
+%% unp_connect	vp	L L L
+
+vop_unp_connect {
 	IN struct vnode *vp;
+	OUT struct socket **socket;
 };
 
-vop_spare3 {
+%% unp_detach	vp	= = =
+
+vop_unp_detach {
 	IN struct vnode *vp;
 };
 
+# The VOPs below are spares at the end of the table to allow new VOPs to be
+# added in stable branches without breaking the KBI.  New VOPs in HEAD should
+# be added above these spares.  When merging a new VOP to a stable branch,
+# the new VOP should replace one of the spares.
+
 vop_spare4 {
 	IN struct vnode *vp;
 };
Index: sys/kern/uipc_usrreq.c
===================================================================
--- sys/kern/uipc_usrreq.c	(revision 234659)
+++ sys/kern/uipc_usrreq.c	(revision 234660)
@@ -541,7 +541,7 @@
 
 	UNP_LINK_WLOCK();
 	UNP_PCB_LOCK(unp);
-	vp->v_socket = unp->unp_socket;
+	VOP_UNP_BIND(vp, unp->unp_socket);
 	unp->unp_vnode = vp;
 	unp->unp_addr = soun;
 	unp->unp_flags &= ~UNP_BINDING;
@@ -637,7 +637,7 @@
 	 * XXXRW: Should assert vp->v_socket == so.
 	 */
 	if ((vp = unp->unp_vnode) != NULL) {
-		unp->unp_vnode->v_socket = NULL;
+		VOP_UNP_DETACH(vp);
 		unp->unp_vnode = NULL;
 	}
 	unp2 = unp->unp_conn;
@@ -1307,7 +1307,7 @@
 	 * and to protect simultaneous locking of multiple pcbs.
 	 */
 	UNP_LINK_WLOCK();
-	so2 = vp->v_socket;
+	VOP_UNP_CONNECT(vp, &so2);
 	if (so2 == NULL) {
 		error = ECONNREFUSED;
 		goto bad2;
@@ -2317,17 +2317,15 @@
 
 	active = 0;
 	UNP_LINK_WLOCK();
-	so = vp->v_socket;
+	VOP_UNP_CONNECT(vp, &so);
 	if (so == NULL)
 		goto done;
 	unp = sotounpcb(so);
 	if (unp == NULL)
 		goto done;
 	UNP_PCB_LOCK(unp);
-	if (unp->unp_vnode != NULL) {
-		KASSERT(unp->unp_vnode == vp,
-		    ("vfs_unp_reclaim: vp != unp->unp_vnode"));
-		vp->v_socket = NULL;
+	if (unp->unp_vnode == vp) {
+		VOP_UNP_DETACH(vp);
 		unp->unp_vnode = NULL;
 		active = 1;
 	}
Index: sys/kern/vfs_default.c
===================================================================
--- sys/kern/vfs_default.c	(revision 234659)
+++ sys/kern/vfs_default.c	(revision 234660)
@@ -123,6 +123,9 @@
 	.vop_unlock =		vop_stdunlock,
 	.vop_vptocnp =		vop_stdvptocnp,
 	.vop_vptofh =		vop_stdvptofh,
+	.vop_unp_bind =		vop_stdunp_bind,
+	.vop_unp_connect =	vop_stdunp_connect,
+	.vop_unp_detach =	vop_stdunp_detach,
 };
 
 /*
@@ -1037,6 +1040,30 @@
 	return (error);
 }
 
+int
+vop_stdunp_bind(struct vop_unp_bind_args *ap)
+{
+
+	ap->a_vp->v_socket = ap->a_socket;
+	return (0);
+}
+
+int
+vop_stdunp_connect(struct vop_unp_connect_args *ap)
+{
+
+	*ap->a_socket = ap->a_vp->v_socket;
+	return (0);
+}
+
+int
+vop_stdunp_detach(struct vop_unp_detach_args *ap)
+{
+
+	ap->a_vp->v_socket = NULL;
+	return (0);
+}
+
 /*
  * vfs default ops
  * used to fill the vfs function table to get reasonable default return values.
Index: sys/sys/vnode.h
===================================================================
--- sys/sys/vnode.h	(revision 234659)
+++ sys/sys/vnode.h	(revision 234660)
@@ -707,6 +707,9 @@
 int	vop_stdpoll(struct vop_poll_args *);
 int	vop_stdvptocnp(struct vop_vptocnp_args *ap);
 int	vop_stdvptofh(struct vop_vptofh_args *ap);
+int	vop_stdunp_bind(struct vop_unp_bind_args *ap);
+int	vop_stdunp_connect(struct vop_unp_connect_args *ap);
+int	vop_stdunp_detach(struct vop_unp_detach_args *ap);
 int	vop_eopnotsupp(struct vop_generic_args *ap);
 int	vop_ebadf(struct vop_generic_args *ap);
 int	vop_einval(struct vop_generic_args *ap);

Property changes on: sys
___________________________________________________________________
Modified: svn:mergeinfo
   Merged /head/sys:r232317


Reply to: