Re: [PATCH] tmpfs-update 021010 (was: tmpfs work update 013010)

[Naoya Sugioka <naoya.sugioka@xxxxxxxxx>]

On 2/12/10, Matthew Dillon <dillon@apollo.backplane.com> wrote:
> :Unfortunately, this patch is not stable with current master head. Let
> :me explain first.
> :I was issuing fsstress command (ex fsstress -p 100 -n 100 -d XXX )
> :then I was trying to
> :solve filesystem crash issue. But the situation got unexpected from
> :two days ago.
> :>From two days ago and now, it starts showing unexpected filesystem
> :issue is happening
> :(and it is why I would post this even if this is not stable). I need
> :suggestions, clues or any
> :advice to move forward.
> :
> :What I can observe is:
> :
> :1) mount /usr/src /usr/obj as tmpfs
> :2) untar src-sys.tgz (from release 2.4.1 image)
> :3) run 'make nativekernel'
> :4) build stopped somewhere with compile error
> :5) but the file seems correct to me.
> :6) re-run 'make nativekernel'
> :7) failed again with 'not found' error
> :8) now tmpfs on /usr/src and /usr/obj disappears
> :    they are told as / (root) mounted filesystems. very strange
> :
> :I'm guessing some 'swapcache' related change might cause this issue.
> :Am I missing something
> :very important code change ?
> :
> :Any suggestions are welcome, always.
> :
> :thank you,
> :-Naoya
>
>     Nothing major changed but because tmpfs uses the SWAP strategy
>     code there could certainly be some issues, such as swapcache-related
>     code removing swap assignments when it shouldn't.
>
>     I think its well enough developed that I should go ahead and commit
>     what you have so far, and then I'll take an hour or two and run
>     through FSX and fix whatever low-hanging fruit I can find.  We'll
>     see where we stand at the end of the day.  My take is that whatever
>     bugs are in there are going to be minor and pretty easy to fix.
>
> 						-Matt
>
>

Wow, that's a surprise.  I was expected I need to polish up them, but
I'm glad you accept the patch.

Anyway, attachment is a short-hand patch to raise stability of tmpfs
Please see and checkin.

I can see 'make native' command works with this on the tmpfs mounted /usr/obj.

thank you,
-Naoya

diff --git a/sys/vfs/tmpfs/tmpfs_subr.c b/sys/vfs/tmpfs/tmpfs_subr.c
index 93329b1..eb0aaec 100644
--- a/sys/vfs/tmpfs/tmpfs_subr.c
+++ b/sys/vfs/tmpfs/tmpfs_subr.c
@@ -150,9 +150,9 @@ tmpfs_alloc_node(struct tmpfs_mount *tmp, enum vtype type,
 		break;
 
 	case VLNK:
-		KKASSERT((strlen(target) +1) < MAXPATHLEN);
-		nnode->tn_size = strlen(target) +1;
-		nnode->tn_link = kmalloc(nnode->tn_size, M_TMPFSNAME,
+		nnode->tn_size = strlen(target);
+		KKASSERT(nnode->tn_size <= MAXPATHLEN -1);
+		nnode->tn_link = kmalloc(nnode->tn_size + 1, M_TMPFSNAME,
 		    M_WAITOK);
 		bcopy(target, nnode->tn_link, nnode->tn_size);
 		nnode->tn_link[nnode->tn_size] = '\0';
@@ -242,6 +242,8 @@ tmpfs_free_node(struct tmpfs_mount *tmp, struct tmpfs_node *node)
 
 	case VLNK:
 		kfree(node->tn_link, M_TMPFSNAME);
+		node->tn_link = NULL;
+		node->tn_size = 0;
 		break;
 
 	case VREG:
@@ -282,7 +284,7 @@ tmpfs_alloc_dirent(struct tmpfs_mount *tmp, struct tmpfs_node *node,
 
 
 	nde = (struct tmpfs_dirent *)objcache_get(tmp->tm_dirent_pool, M_WAITOK);
-	nde->td_name = kmalloc(len, M_TMPFSNAME, M_WAITOK);
+	nde->td_name = kmalloc(len + 1, M_TMPFSNAME, M_WAITOK);
 	nde->td_namelen = len;
 	bcopy(name, nde->td_name, len);
 	nde->td_name[len] = '\0';
@@ -627,6 +629,7 @@ tmpfs_dir_lookup(struct tmpfs_node *node, struct tmpfs_node *f,
 	TMPFS_VALIDATE_DIR(node);
 
 	found = 0;
+	crit_enter();
 	TAILQ_FOREACH(de, &node->tn_dir.tn_dirhead, td_entries) {
 		if (f != NULL && de->td_node != f)
 		    continue;
@@ -637,6 +640,7 @@ tmpfs_dir_lookup(struct tmpfs_node *node, struct tmpfs_node *f,
 			}
 		}
 	}
+	crit_exit();
 
 	TMPFS_NODE_LOCK(node);
 	node->tn_status |= TMPFS_NODE_ACCESSED;
@@ -679,7 +683,9 @@ tmpfs_dir_getdotdent(struct tmpfs_node *node, struct uio *uio)
 			uio->uio_offset = TMPFS_DIRCOOKIE_DOTDOT;
 	}
 
+	TMPFS_NODE_LOCK(node);
 	node->tn_status |= TMPFS_NODE_ACCESSED;
+	TMPFS_NODE_UNLOCK(node);
 
 	return error;
 }
@@ -728,15 +734,19 @@ tmpfs_dir_getdotdotdent(struct tmpfs_node *node, struct uio *uio)
 		if (error == 0) {
 			struct tmpfs_dirent *de;
 
+			crit_enter();
 			de = TAILQ_FIRST(&node->tn_dir.tn_dirhead);
 			if (de == NULL)
 				uio->uio_offset = TMPFS_DIRCOOKIE_EOF;
 			else
 				uio->uio_offset = tmpfs_dircookie(de);
+			crit_exit();
 		}
 	}
 
+	TMPFS_NODE_LOCK(node);
 	node->tn_status |= TMPFS_NODE_ACCESSED;
+	TMPFS_NODE_UNLOCK(node);
 
 	return error;
 }
@@ -756,11 +766,13 @@ tmpfs_dir_lookupbycookie(struct tmpfs_node *node, off_t cookie)
 		return node->tn_dir.tn_readdir_lastp;
 	}
 
+	crit_enter();
 	TAILQ_FOREACH(de, &node->tn_dir.tn_dirhead, td_entries) {
 		if (tmpfs_dircookie(de) == cookie) {
 			break;
 		}
 	}
+	crit_exit();
 
 	return de;
 }
@@ -800,6 +812,7 @@ tmpfs_dir_getdents(struct tmpfs_node *node, struct uio *uio, off_t *cntp)
 
 	/* Read as much entries as possible; i.e., until we reach the end of
 	 * the directory or we exhaust uio space. */
+	crit_enter();
 	do {
 		struct dirent d;
 		int reclen;
@@ -860,6 +873,7 @@ tmpfs_dir_getdents(struct tmpfs_node *node, struct uio *uio, off_t *cntp)
 		(*cntp)++;
 		de = TAILQ_NEXT(de, td_entries);
 	} while (error == 0 && uio->uio_resid > 0 && de != NULL);
+	crit_exit();
 
 	/* Update the offset and cache. */
 	if (de == NULL) {
@@ -870,7 +884,10 @@ tmpfs_dir_getdents(struct tmpfs_node *node, struct uio *uio, off_t *cntp)
 		node->tn_dir.tn_readdir_lastn = uio->uio_offset = tmpfs_dircookie(de);
 		node->tn_dir.tn_readdir_lastp = de;
 	}
+
+	TMPFS_NODE_LOCK(node);
 	node->tn_status |= TMPFS_NODE_ACCESSED;
+	TMPFS_NODE_UNLOCK(node);
 
 	return error;
 }
diff --git a/sys/vfs/tmpfs/tmpfs_vnops.c b/sys/vfs/tmpfs/tmpfs_vnops.c
index 21088ec..edd1e50 100644
--- a/sys/vfs/tmpfs/tmpfs_vnops.c
+++ b/sys/vfs/tmpfs/tmpfs_vnops.c
@@ -485,7 +485,7 @@ tmpfs_read (struct vop_read_args *ap)
 
 	if (uio->uio_offset < 0)
 		return (EINVAL);
-	if (vp->v_type != VREG)
+	if (vp->v_type != VREG) 
 		return (EINVAL);
 	
 	vn_lock(vp, LK_SHARED | LK_RETRY);
@@ -792,9 +792,9 @@ tmpfs_nremove(struct vop_nremove_args *v)
 	        TMPFS_NODE_UNLOCK(node);
 	}
 
+	cache_inval_vp(vp, CINV_DESTROY);
 	cache_setunresolved(v->a_nch);
 	cache_setvp(v->a_nch, NULL);
-	cache_inval_vp(vp, CINV_DESTROY);
 	error = 0;
 
 
@@ -984,7 +984,7 @@ tmpfs_nrename(struct vop_nrename_args *v)
 	 * has to be changed. */
 	if (fncp->nc_nlen != tncp->nc_nlen ||
 	    bcmp(fncp->nc_name, tncp->nc_name, fncp->nc_nlen) != 0) {
-		newname = kmalloc(tncp->nc_nlen, M_TMPFSNAME, M_WAITOK);
+		newname = kmalloc(tncp->nc_nlen + 1, M_TMPFSNAME, M_WAITOK);
 	} else
 		newname = NULL;
 
@@ -1235,9 +1235,9 @@ tmpfs_nrmdir(struct vop_nrmdir_args *v)
 	TMPFS_NODE_UNLOCK(dnode);
 	tmpfs_update(dvp);
 
+	cache_inval_vp(vp, CINV_DESTROY);
 	cache_setunresolved(v->a_nch);
 	cache_setvp(v->a_nch, NULL);
-	cache_inval_vp(vp, CINV_DESTROY);
 	error = 0;
 
 out:
@@ -1332,6 +1332,7 @@ outok:
 		*ncookies = cnt;
 		*cookies = kmalloc(cnt * sizeof(off_t), M_TEMP, M_WAITOK);
 
+		crit_enter();
 		for (i = 0; i < cnt; i++) {
 			KKASSERT(off != TMPFS_DIRCOOKIE_EOF);
 			if (off == TMPFS_DIRCOOKIE_DOT) {
@@ -1355,6 +1356,7 @@ outok:
 
 			(*cookies)[i] = off;
 		}
+		crit_exit();
 		KKASSERT(uio->uio_offset == off);
 	}
 	vn_unlock(vp);