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

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

hi,

> On Fri, Aug 19, 2011 at 01:54:28PM +0000, Emmanuel Dreyfus wrote:
>> Here is my complete anlysis of the problem:
> (snip)

your analysis seems to make a sense.

> 
> And here is a fix in puffs_vnop_getattr(). It was tested on netbsd-5 only so 
> far since the bug does not appear in -current. My test case does not exhibit
> the bug anymore on netbsd-5.
> 
> Opinions?
> 
> I obtain this without yamt's patch that flushes writes in dosetattr()
> before sending the SETATTR message. I wonder if this is really a 
> different problem. yamt, how do you get the problem that is fixed by
> your patch? I would like to check if I can reproduce.

sorry, i forgot.

> 
> 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 4 -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  19 Aug 2011 15:37:53 -0000
> @@ -897,8 +897,21 @@
>                 vap->va_size = pn->pn_mc_size;
>         } else {
>                 if (rvap->va_size != VNOVAL
>                     && vp->v_type != VBLK && vp->v_type != VCHR) {
> +                       /*
> +                        * There may be a pending SETATTR triggered by
> +                        * another process (e.g.: syncer). The kernel
> +                        * is waiting in dosetattr() and decided on
> +                        * a newer and bigger size. If we report a
> +                        * smaller and older size from the filesystem,
> +                        * uvm_vnp_setsize() will truncate the file and
> +                        * discard the cached data not yet written. We
> +                        * therefore retain the biggest value from 
> +                        * kernel and filesystem to avoid data corruption.
> +                        */
> +                       rvap->va_size = MAX(rvap->va_size, vp->v_size);
> +

couldn't it be a problem if the size of the file is shrinked by the server?

how about stopping using FAF for SETATTR?
i guess puffs_msg_setcall can be used instead.

YAMAMOTO Takashi

>                         uvm_vnp_setsize(vp, rvap->va_size);
>                         pn->pn_serversize = rvap->va_size;
>                 }
>         }
> 
> 
> -- 
> Emmanuel Dreyfus
> manu%netbsd.org@localhost