Re: CVS commit: src/sys/kern

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

hi,

> 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?

round_page(offset) < trunc_page(endoffset)

YAMAMOTO Takashi

> 
>> +                    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: