Re: CVS commit: src/sys/kern

[Masao Uebayashi <uebayasi%tombi.co.jp@localhost>]

On Thu, Nov 11, 2010 at 02:10:00PM +0000, YAMAMOTO Takashi wrote:
> hi,
> 
> > Module Name:        src
> > Committed By:       rmind
> > Date:               Wed Oct 27 02:58:05 UTC 2010
> > 
> > Modified Files:
> >     src/sys/kern: sys_descrip.c
> > 
> > Log Message:
> > do_posix_fadvise: check for a negative length; truncate the offset and
> > round the end-offset, not vice-versa.
> 
> the latter part seems incorrect and makes putpages panic.
> note that round_page(INT64_MAX) is negative.
> 
> how about the attached patch?
> 
> YAMAMOTO Takashi
> 
> > 
> > Thanks to jakllsch@ for debug info.
> > 
> > 
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.17 -r1.18 src/sys/kern/sys_descrip.c
> > 
> > Please note that diffs are not public domain; they are subject to the
> > copyright notices on the relevant files.

> Index: sys_descrip.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/sys_descrip.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 sys_descrip.c
> --- sys_descrip.c     27 Oct 2010 02:58:04 -0000      1.18
> +++ sys_descrip.c     11 Nov 2010 14:06:40 -0000
> @@ -680,9 +680,21 @@ do_posix_fadvise(int fd, off_t offset, o
>  
>       case POSIX_FADV_DONTNEED:
>               vp = fp->f_data;
> -             mutex_enter(&vp->v_interlock);
> -             error = VOP_PUTPAGES(vp, trunc_page(offset),
> -                 round_page(endoffset), PGO_DEACTIVATE | PGO_CLEANIT);
> +             /*
> +              * align the region to page boundaries as VOP_PUTPAGES expects
> +              * by shrinking it.  we shrink instead of expand because we
> +              * don't want to deactivate cache outside of the requested
> +              * region.  it means that, if the specified region is smaller
> +              * than PAGE_SIZE, we do nothing.
> +              */
> +             if (endoffset - offset >= PAGE_SIZE) {

What is the intended requirement of round_page(offset) and
trunc_page(endoffset) at this point?

> +                     mutex_enter(&vp->v_interlock);
> +                     error = VOP_PUTPAGES(vp, round_page(offset),
> +                         trunc_page(endoffset),
> +                         PGO_DEACTIVATE | PGO_CLEANIT);
> +             } else {
> +                     error = 0;
> +             }
>               break;
>  
>       case POSIX_FADV_NOREUSE: