Re: [PATCH] Re: zero-filed page on VOP_PUTPAGES

[yamt%mwd.biglobe.ne.jp@localhost (YAMAMOTO Takashi)]

hi,

> Emmanuel Dreyfus <manu%netbsd.org@localhost> 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
> manu%netbsd.org@localhost