hi, > Emmanuel Dreyfus <[email protected]> wrote: > >> > > + rvap->va_size = MAX(rvap->va_size, vp->v_size); >> I am back to this simple fix > > Here is an alternative approach that works around the data corruption > bug, it seems much cleaner to me.
i don't think it fixes the problem as VOPs can release the lock before the completion of async PUFFS_VN_SETATTR. besides that, non-diagnostic uses of VOP_ISLOCKED are mostly considered bugs. YAMAMOTO Takashi > > It does not address the crash on low memory situation, but as I said > this one is a second different bug. > > 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 25 Aug 2011 22:56:27 -0000 > @@ -855,6 +855,18 @@ puffs_vnop_getattr(void *v) > struct vattr *vap, *rvap; > struct puffs_node *pn = VPTOPP(vp); > int error = 0; > + int locked; > + > + /* > + * 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). > + */ > + if ((locked = VOP_ISLOCKED(vp)) != LK_EXCLUSIVE) > + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); > > REFPN(pn); > vap = ap->a_vap; > @@ -906,6 +918,10 @@ puffs_vnop_getattr(void *v) > out: > puffs_releasenode(pn); > PUFFS_MSG_RELEASE(getattr); > + > + if (locked != LK_EXCLUSIVE) > + VOP_UNLOCK(vp, 0); > + > return error; > } > > > -- > Emmanuel Dreyfus > http://hcpnet.free.fr/pubz > [email protected]
