Subject: Re: genfs_getpages() nit
To: Bill Studenmund <wrstuden@zembu.com>
From: Jaromír Dolecek <dolecek@ics.muni.cz>
List: tech-kern
Date: 01/27/2001 11:24:08
> > - endoffset = round_page((origoffset + (npages << PAGE_SHIFT)
> > - + fs_bsize - 1) & ~(fs_bsize - 1));
> > + if (fs_bshift < PAGE_SHIFT)
> > + endoffset = round_page(startoffset + (npages << PAGE_SHIFT));
> > + else {
> > + endoffset = round_page(startoffset +
> > + (((npages << PAGE_SHIFT) + fs_bsize - 1) & ~(fs_bsize - 1)));
> > + }
>
> "I'm-still-confused"
If I read the code correctly, this code tries to make endoffset rounded to both
PAGE_SIZE and fs_bsize. If fs_bshift is < PAGE_SHIFT, we don't need
to count with fs_bsize, since it would not have any influence on
end result. Also, we can use startoffset instead of origoffset, since
that is already rounded to fs_bsize.
> > endoffset = min(endoffset, round_page(eof));
> > - ridx = (origoffset - startoffset) >> PAGE_SHIFT;
> > + ridx = (origoffset & fs_bsize) >> PAGE_SHIFT;
>
> Shouldn't that be ridx = (origoffset & (fs_bsize - 1)) >> PAGE_SHIFT;
> ?
Ah, true. I'm not sure which one is more readable then (the old one,
or this new one).
I also realized two other thigs which might be "interesting" from
looking at genfs_getpages(). Maybe I'm just confused (which is quite likely ;)
* does mmap() work if filesystem block size is >= 16 * PAGE_SIZE in UBC
in all cases ?
* I'm not sure if the code in genfs_getpages() below uvm_pagermapin()
(line approx. 628) is okay for block size > PAGE_SIZE; shouldn't
the offset ridx be used there as well similarily to code above it ?
i.e. like
kva = uvm_pagermapin(&pgs[ridx], npages, ...)
Jaromir
--
Jaromir Dolecek <jdolecek@NetBSD.org> http://www.ics.muni.cz/~dolecek/
@@@@ Wanna a real operating system ? Go and get NetBSD, dammit! @@@@