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

[Thor Lancelot Simon <tls%panix.com@localhost>]

On Tue, Oct 09, 2012 at 05:35:44PM +0200, Manuel Bouyer wrote:
> 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 ...)

But the chunk-size really has two effects: we round short transfers *up*
to (a multiple of) the chunk-size if the file is large enough
(because intermediate layers lose the actual requested I/O size even
for read(), again IIRC) and we break large readheads *down* into multiple
reads of chunk-size.

That's the reason for the somewhat odd definition.  So I think the power
of 2 check should in fact go, now that I've thought through it more.

It seems to me if we got the residual at the end of the file right before
we probably still do now, but that could definitely use another set of
eyes.

Thor