Page 1 of 1
I found a bug in the wiki
Posted: Tue Feb 04, 2014 7:09 pm
by Luca91
Hi,
I was trying to write the heap (well, I was trying to port port liballoc), and I found this interesting article that explain how to find a number contiguos pages:
http://wiki.osdev.org/User:Kmcguire/Qui ... Free_Pages
It was an interesting read, but I found 2 issue with that code:
Code: Select all
unsigned int vmm_findfree(unsigned int *vdir, unsigned int count, unsigned int index4mb, unsigned int icount4mb){
unsigned int *table;
unsigned int x;
unsigned int cfreepage = 0;
unsigned int opage = index4mb << 22;
for(x = index4mb; x < (index4mb+icount4mb) ; ++x){
if(vdir[x] != 0){
table = (unsigned int*)(vdir[x] & 0xFFFFF000);
for(y = 0; y < 1024; ++y){
if(table[y] != 0){
++cfreepage;
if(cfreepage >= count){
return opage;
}
}else{
// reset the count
cfreepage = 0;
// this should end up being the first page notice the (y+1) and currently y=y+0.
opage = (x << 22) + ((y+1) << 12);
}
}
}else{
// count completely blank tables
cfreepage += 1024;
}
}
return 0;
}
well, 2 of the 3 problem are in this part of the code:
Code: Select all
}else{
// count completely blank tables
cfreepage += 1024;
}
}
return 0;
}
let's assume that the actual address is not in a table, 1024 will be added to cfreepage, but next iteration the same address + 0x1 will not be in a table (ofcourse!) but then another 1024 will be added to cfreepage!!
I fixed it by adding 0x1000 (the size of a pagetable) to x when a balnk table is found... in that way:
Code: Select all
}else{
// count completely blank tables
cfreepage += 1024;
x+=0x1000;
}
}
return 0;
}
Another problem is that if there isn't an valid pagetable in the search range, the function will not return a valid starting address..
I fixed it in that way:
Code: Select all
}else{
// count completely blank tables
cfreepage += 1024;
x+=0x1000;
if(cfreepage >= count){
return opage;
}
Ok, what do you think about ? are those fixes valid and stables ?
Thank you very much
EDIT: I found another bug, when you try to find just 1 free page, it magically always find it (even if the given range is full of used pages)
Re: I found a bug in the wiki
Posted: Tue Feb 04, 2014 8:01 pm
by sortie
Found a bug in the wiki? Correct it!
Seriously, though. That's a page in the user namespace. It's not an official article, but just a place where users can locate their temporary work. It's somewhat rude to modify stuff that's inside someone's private namespace. In this case, I've actually never seen that user article before and it looks somewhat oldish. I'll be sure to review this more closely soonish, though.
Re: I found a bug in the wiki
Posted: Tue Feb 04, 2014 8:15 pm
by Luca91
Thank you very much sortie, if you eventually manage to find a fix to get a propper way to find contiguous pages, please let me know, as I'm trying hard to achive it..
Re: I found a bug in the wiki
Posted: Wed Feb 05, 2014 12:16 am
by jnc100
Looking at the history the reason this was moved to the users personal namespace was presumably because they did not agree to the CC0 license for it. You are therefore on reasonably shaky ground legally if you copy it to your OS. We should probably delete it...
Regards,
John.
Re: I found a bug in the wiki
Posted: Wed Feb 05, 2014 1:21 am
by Combuster
Kevin McGuire did agree to the relicensing, but the copyright issue predates that. Now if I could remember what the actual issue was because it's not documented anywhere I might think of a more appropriate fix, but it looks like deletion will have to happen.
Re: I found a bug in the wiki
Posted: Wed Feb 05, 2014 7:50 am
by Kevin
Luca91 wrote:let's assume that the actual address is not in a table, 1024 will be added to cfreepage, but next iteration the same address + 0x1 will not be in a table (ofcourse!) but then another 1024 will be added to cfreepage!!
I fixed it by adding 0x1000 (the size of a pagetable) to x when a balnk table is found... in that way:
Did you try it out, i.e. you reproduced a bug and tested that your fix fixes it? Because x doesn't look like a page number, but rather like a page directory index. I'm pretty sure your "fix" breaks the function.
Re: I found a bug in the wiki
Posted: Wed Feb 05, 2014 8:33 am
by Luca91
Kevin wrote:Luca91 wrote:let's assume that the actual address is not in a table, 1024 will be added to cfreepage, but next iteration the same address + 0x1 will not be in a table (ofcourse!) but then another 1024 will be added to cfreepage!!
I fixed it by adding 0x1000 (the size of a pagetable) to x when a balnk table is found... in that way:
Did you try it out, i.e. you reproduced a bug and tested that your fix fixes it? Because x doesn't look like a page number, but rather like a page directory index. I'm pretty sure your "fix" breaks the function.
0x1000 is the size of a pagetable, so adding 0x1000 to the page dir index, I should arrive to the next pagetable..
By the way the functions doesn't works correctly due to other porblems (with opages, and due to the fact that it always find an empy page when the requested number of pages is 1)..
I'm still trying to find a proper way to find n contiguous free pages
Re: I found a bug in the wiki
Posted: Thu Feb 06, 2014 1:35 pm
by Pancakes
I do not have the time to fix it, but I release all copyrights to the material. I thought I had already released all rights, but I can do it again, LOL.
And, you can use it in your OS it is not anything anyone in their right mind would copyright, LOL. I mean if you consider basic programming able to be copyrighted. =P
Re: I found a bug in the wiki
Posted: Thu Feb 06, 2014 2:55 pm
by Kevin
Luca91 wrote:0x1000 is the size of a pagetable, so adding 0x1000 to the page dir index, I should arrive to the next pagetable..
No. A page directory entry is 32 bits, not 4096 bytes. It's the difference between a data structure itself and a pointer to it.
Re: I found a bug in the wiki
Posted: Fri Feb 07, 2014 3:03 am
by bwat
Pancakes wrote:And, you can use it in your OS it is not anything anyone in their right mind would copyright, LOL. I mean if you consider basic programming able to be copyrighted. =P
Check out the implementations of
true for Irix and Solaris here
http://melkfl.es/true/source