Re: [9fans] kernel bug
erik quanstrom once said: > i don't know where a history of stuff older than sources (2002) is. /n/sources/extra/9hist Anthony
Re: [9fans] kernel bug
Looking at the diffs I can see how it might easily be missed, since there are many other changes to allow for different page sizes. It's different again in the 64-bit version, where I really went over the top (nice 1914 centenary reference there) on changes. I'd better put that code somewhere before someone shouts at me.
Re: [9fans] kernel bug
On 3 June 2014 18:08, erik quanstrom wrote: > ok. good. that's what happened. perhaps some change needs > to be made to segattach to mirror this change. > I think this is independent. In ibrk I changed the test from using newsize to calculating mapsize first and checking mapsize in the same way as above, since mapsize had to be calculated anyway, and it seemed better to keep the tests the same.
Re: [9fans] kernel bug
> I made the change you suggest in the PAE kernel but perhaps Erik missed it
> during his merge:
> if(mapsize > nelem(s->ssegmap)){
> mapsize *= 2;
> if(mapsize > SEGMAPSIZE)
> mapsize = SEGMAPSIZE;
> s->map = smalloc(mapsize*sizeof(Pte*));
> s->mapsize = mapsize;
> }
ok. good. that's what happened. perhaps some change needs
to be made to segattach to mirror this change.
- erik
Re: [9fans] kernel bug
On 3 June 2014 02:14, Yoann Padioleau wrote:
> if(mapsize > (SEGMAPSIZE*PTEPERTAB))
> mapsize = (SEGMAPSIZE*PTEPERTAB);
>
...
>
>
I made the change you suggest in the PAE kernel but perhaps Erik missed it
during his merge:
if(mapsize > nelem(s->ssegmap)){
mapsize *= 2;
if(mapsize > SEGMAPSIZE)
mapsize = SEGMAPSIZE;
s->map = smalloc(mapsize*sizeof(Pte*));
s->mapsize = mapsize;
}
Re: [9fans] kernel bug
too bad, i don't see anthony's diff, and i get this error (perhaps unrelated) Too many diffs (26 > 25). Stopping. - erik
Re: [9fans] kernel bug
> i don't know where a history of stuff older than sources (2002) is. http://swtch.com/plan9history/
Re: [9fans] kernel bug
> I think it should be
> if(mapsize > (SEGMAPSIZE))
> mapsize = SEGMAPSIZE;
hmm. i think this code is correct. ssegmap is a static map
to handle small segments. small segments fit in ssegmap.
the point must have been to avoid malloc.
this test is a little more questionable
> if(mapsize > (SEGMAPSIZE*PTEPERTAB))
> mapsize = (SEGMAPSIZE*PTEPERTAB);
cf. the check in ibrk
if(newsize > (SEGMAPSIZE*PTEPERTAB)) {
qunlock(&s->lk);
error(Enovmem);
}
i think this check is either not wrong, or more extensive rework
is necessary.
@anthony, do you know if this code or similar occurred in even older kernels?
if there was a cap also in ibrk() then i would suspect this code was originally
correct.
i don't know where a history of stuff older than sources (2002) is.
> Also why in the kernel they use 'struct Pte' instead of the better name
> Pagetable.
> In many places this is very confusing because when I see Pte I think of a
> Pagetable Entry
> where really they are speaking about a Pagetable.
i would naturally think a Pte* would be an array of Pte's, i.e. a Pte table,
just like an array of uchar* could be used as a table.
- erik
Re: [9fans] kernel bug
Yoann Padioleau once said:
> in the newseg() function there is:
>
> [...]
>
> I think it should be
> if(mapsize > (SEGMAPSIZE))
> mapsize = SEGMAPSIZE;
Yes, you're correct.
The code allows the creation of a segment VM map with more
than SEGMAPSIZE Ptes.
Personally, I'd remove the check entirely along with the
optimization of doubling the segment size, perhaps moving
it to ibrk. I think it's more likely that the segment will
grow if brk is called at least once.
Anthony
P.S. It looks like this is a 15 year old bug:
1998/0916/sys/src/9/port/segment.c:62,67 -
1998/0919/sys/src/9/port/segment.c:62,70
mapsize = ROUND(size, PTEPERTAB)/PTEPERTAB;
if(mapsize > nelem(s->ssegmap)){
+ mapsize *= 2;
+ if(mapsize > (SEGMAPSIZE*PTEPERTAB))
+ mapsize = (SEGMAPSIZE*PTEPERTAB);
s->map = smalloc(mapsize*sizeof(Pte*));
s->mapsize = mapsize;
}
Re: [9fans] kernel bug
good catch. thank you. -- cinap
