Re: CVS commit: [tls-maxphys] src/sys/dev

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Tue, Oct 09, 2012 at 11:24:13AM -0400, Thor Lancelot Simon wrote:
> On Tue, Oct 09, 2012 at 05:21:17PM +0200, Manuel Bouyer wrote:
> > On Tue, Oct 09, 2012 at 11:04:20AM -0400, Thor Lancelot Simon wrote:
> > > Hmmmmm.
> > > 
> > > This will be an attempt to transfer an entire file -- 139264 will be the
> > > entire file size of something, probably an executable it's paging in.
> > > 
> > > I saw this before -- I can't remember what caused it -- the "bad 
> > > chunksize"
> > > is, I think, because it's not power-of-2.  The cause will be in the UVM
> > > readahead code.
> > 
> > It is. It looks like this code is different in tls-maxphys and HEAD.
> 
> Is it different in tls-maxphys-base and HEAD?  I played with this code
> quite a bit while debugging previously, I believe.

Well, the code which cause the panic changed from
        const size_t chunksize = RA_IOCHUNK;
to
        const size_t chunksize = MIN(chunksz, round_page(sz));

so the power-of-2 check changes from checking that a #define constant
has the right value, to sanity-checking that the values we got from upper
layers. And AFAIK then callers don't make any effort for these values to
be a power-of-2, but it may make sure that it's page-size aligned
(I'm not even sure of that: the end of the file is not necesserely
page-aligned ...)

> 
> > I wonder if we really want to read in power-of-2 chunks here, or just
> > multiple of page size. Reading the code which calls ra_startio(),
> > I'm not sure why size of chunksize would be a power-of-2.
> 
> I wondered that myself.  I think multiple of pagesize is probably okay,
> but -- IIRC -- I couldn't convince myself we'd always do the right thing
> with the residual.  Try it?

I'll have a closer look.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--