On Fri, Aug 26, 2011 at 08:33:47AM +0000, YAMAMOTO Takashi wrote:
> if we go this route, it's cleaner to use a puffs internal lock
I did it that way, and it seems to work fine.
--
Emmanuel Dreyfus
m...@netbsd.org
? sys/fs/puffs/puffs_vnops.c-debug
? sys/fs/puffs/puffs_vnops.c.ok1
? sys/fs/puffs/puffs_vnops.c.ok2
Index: sys/fs/puffs/puffs_node.c
===================================================================
RCS file: /cvsroot/src/sys/fs/puffs/puffs_node.c,v
retrieving revision 1.13.10.1
diff -u -p -d -r1.13.10.1 puffs_node.c
--- sys/fs/puffs/puffs_node.c 3 Oct 2009 23:11:27 -0000 1.13.10.1
+++ sys/fs/puffs/puffs_node.c 26 Aug 2011 15:18:57 -0000
@@ -155,6 +155,7 @@ puffs_getvnode(struct mount *mp, puffs_c
}
KASSERT(pnc != NULL);
}
+ mutex_init(&pnode->pn_sizemtx, MUTEX_DEFAULT, IPL_NONE);
mutex_exit(&pmp->pmp_lock);
vp->v_data = pnode;
Index: sys/fs/puffs/puffs_sys.h
===================================================================
RCS file: /cvsroot/src/sys/fs/puffs/puffs_sys.h,v
retrieving revision 1.70.20.2
diff -u -p -d -r1.70.20.2 puffs_sys.h
--- sys/fs/puffs/puffs_sys.h 18 Jun 2011 16:19:39 -0000 1.70.20.2
+++ sys/fs/puffs/puffs_sys.h 26 Aug 2011 15:18:57 -0000
@@ -205,6 +205,8 @@ struct puffs_node {
voff_t pn_serversize;
struct lockf * pn_lockf;
+ kmutex_t pn_sizemtx; /* size modification mutex */
+
LIST_ENTRY(puffs_node) pn_hashent;
};
Index: sys/fs/puffs/puffs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/fs/puffs/puffs_vnops.c,v
retrieving revision 1.129.4.9
diff -u -p -d -r1.129.4.9 puffs_vnops.c
--- sys/fs/puffs/puffs_vnops.c 17 Jul 2011 15:36:03 -0000 1.129.4.9
+++ sys/fs/puffs/puffs_vnops.c 26 Aug 2011 15:18:58 -0000
@@ -856,6 +856,16 @@ puffs_vnop_getattr(void *v)
struct puffs_node *pn = VPTOPP(vp);
int error = 0;
+ /*
+ * A lock is required so that we do not race with
+ * setattr, write and fsync when changing vp->v_size.
+ * This is critical, since setting a stall smaler value
+ * triggers a file truncate in uvm_vnp_setsize(), which
+ * most of the time means data corruption (a chunk of
+ * data is replaced by zeroes).
+ */
+ mutex_enter(&pn->pn_sizemtx);
+
REFPN(pn);
vap = ap->a_vap;
@@ -906,6 +916,9 @@ puffs_vnop_getattr(void *v)
out:
puffs_releasenode(pn);
PUFFS_MSG_RELEASE(getattr);
+
+ mutex_exit(&pn->pn_sizemtx);
+
return error;
}
@@ -919,6 +932,8 @@ dosetattr(struct vnode *vp, struct vattr
struct puffs_node *pn = vp->v_data;
int error = 0;
+ KASSERT(!(flags & SETATTR_CHSIZE) || mutex_owned(&pn->pn_sizemtx));
+
if ((vp->v_mount->mnt_flag & MNT_RDONLY) &&
(vap->va_uid != (uid_t)VNOVAL || vap->va_gid != (gid_t)VNOVAL
|| vap->va_atime.tv_sec != VNOVAL || vap->va_mtime.tv_sec != VNOVAL
@@ -989,8 +1004,14 @@ puffs_vnop_setattr(void *v)
struct vattr *a_vap;
kauth_cred_t a_cred;
} */ *ap = v;
+ struct puffs_node *pn = ap->a_vp->v_data;
+ int error;
- return dosetattr(ap->a_vp, ap->a_vap, ap->a_cred, SETATTR_CHSIZE);
+ mutex_enter(&pn->pn_sizemtx);
+ error = dosetattr(ap->a_vp, ap->a_vap, ap->a_cred, SETATTR_CHSIZE);
+ mutex_exit(&pn->pn_sizemtx);
+
+ return error;
}
static __inline int
@@ -1040,6 +1061,7 @@ puffs_vnop_inactive(void *v)
int error;
pnode = vp->v_data;
+ mutex_enter(&pnode->pn_sizemtx);
if (doinact(pmp, pnode->pn_stat & PNODE_DOINACT)) {
flushvncache(vp, 0, 0, false);
@@ -1059,6 +1081,7 @@ puffs_vnop_inactive(void *v)
*/
*ap->a_recycle = ((pnode->pn_stat & PNODE_NOREFS) != 0);
+ mutex_exit(&pnode->pn_sizemtx);
VOP_UNLOCK(vp, 0);
return 0;
@@ -1342,13 +1365,15 @@ puffs_vnop_fsync(void *v)
} */ *ap = v;
PUFFS_MSG_VARS(vn, fsync);
struct vnode *vp = ap->a_vp;
+ struct puffs_node *pn = VPTOPP(vp);
struct puffs_mount *pmp = MPTOPUFFSMP(vp->v_mount);
int error, dofaf;
+ mutex_enter(&pn->pn_sizemtx);
error = flushvncache(vp, ap->a_offlo, ap->a_offhi,
(ap->a_flags & FSYNC_WAIT) == FSYNC_WAIT);
if (error)
- return error;
+ goto out;
/*
* HELLO! We exit already here if the user server does not
@@ -1356,8 +1381,9 @@ puffs_vnop_fsync(void *v)
* has references neither in the kernel or the fs server.
* Otherwise we continue to issue fsync() forward.
*/
+ error = 0;
if (!EXISTSOP(pmp, FSYNC))
- return 0;
+ goto out;
dofaf = (ap->a_flags & FSYNC_WAIT) == 0 || ap->a_flags == FSYNC_LAZY;
/*
@@ -1390,6 +1416,8 @@ puffs_vnop_fsync(void *v)
error = checkerr(pmp, error, __func__);
+out:
+ mutex_exit(&pn->pn_sizemtx);
return error;
}
@@ -1932,6 +1960,7 @@ puffs_vnop_write(void *v)
} */ *ap = v;
PUFFS_MSG_VARS(vn, write);
struct vnode *vp = ap->a_vp;
+ struct puffs_node *pn = VPTOPP(vp);
struct puffs_mount *pmp = MPTOPUFFSMP(vp->v_mount);
struct uio *uio = ap->a_uio;
size_t tomove, argsize;
@@ -1943,6 +1972,8 @@ puffs_vnop_write(void *v)
error = uflags = 0;
write_msg = NULL;
+ mutex_enter(&pn->pn_sizemtx);
+
if (vp->v_type == VREG && PUFFS_USE_PAGECACHE(pmp)) {
ubcflags = UBC_WRITE | UBC_PARTIALOK;
if (UBC_WANT_UNMAP(vp))
@@ -2066,6 +2097,7 @@ puffs_vnop_write(void *v)
puffs_msgmem_release(park_write);
}
+ mutex_exit(&pn->pn_sizemtx);
return error;
}
