Understanding JM simple Memory Management

[dc740]

Hi, as some of the users may already know. I'm following the JamesM's kernel development tutorials.

In this occasion I was reading through the Paging chapter. And I found out there was one line I wasn't getting at all.

http://www.jamesmolloy.co.uk/tutorial_h ... aging.html

Code: Select all

When we allocate page tables and directories, they must be page-aligned. So we can build that in:

u32int kmalloc(u32int sz, int align)
{
  if (align == 1 && (placement_address & 0xFFFFF000)) // If the address is not already page-aligned
  {
    // Align it.
    placement_address &= 0xFFFFF000;
    placement_address += 0x1000;
  }
  u32int tmp = placement_address;
  placement_address += sz;
  return tmp;
} 

To check if the address is not already page aligned he does this:
(align == 1 && (placement_address & 0xFFFFF000))

But why placement_address & 0xFFFFF000?
shouldn't it be:
(align == 1 && (placement_address & 0x00000FFF))

so if the placement address is not at the beginning of a 4k page, then we "and" placement_address with 0xFFFFF000 to get it to the beginning and then add 0x1000 to "reserve" the space in the NEXT page?

It may seem like something obvious. But I'm not getting it and I would like to know what is wrong with my logic

[zity]

When you're and'ing an address with 0xFFFFF000 you're setting the last 12 bits to zero. When an address is page aligned, the 12 least significant bits are zero.

Example:
placement_address = 0x12345678
(placement_address & 0xFFFFF000) = 0x12345000

You need to add PAGE_SIZE to be sure you're not overwriting any data between 0x12345000 og 0x12345678.

If you do (placement_address & 0x00000FFF) you get 0x00000678

HTH

[dc740]

I don't know if I didn't understand the answer or if I didn't explain the question enough.


My problem is in the if statement... if align = 1 we must align it...
But why does it check for (placement_address & 0xFFFFF000)?

Example:
placement_address = 0x12345678
(placement_address & 0xFFFFF000) = 0x12345000

the placement address will always return something different than 0. so the code inside the "if" will ALWAYS be executed when align=1... doesn't matter if the placement_address is already aligned.

Let me use your example to explain what I'm trying to say:
placement_address = 0x12345678
(placement_address & 0xFFFFF000) = 0x12345000 <--- attention to this number

that's aligned right? Now lets see what happens when we use an already aligned address and call the functions with align = 1:

placement_address = 0x12345000 <--- special attention here. this address is already aligned as we saw above
(placement_address & 0xFFFFF000) = 0x12345000

So the if statement will be:

align == 1 && (0x12345000 & 0xFFFFF000)) <--- this returns true!!! but there is no need to align the placement_adress cause it's already page aligned. Anyway the JamesM Tutorial gives that code, and that "if" statement ALWAYS executes when align = 1. even when the placement_address is already aligned.

Is it a bug? Am I missing something?

Thanks for your reply

[NickJohnson]

Yeah, that is weird - I think you may have found a bug, because as far as I can tell, it will always return true even if the address is not aligned. Maybe he meant (placement_address & ~0xFFFFF000) or something, although that would just take more space. iirc, there are a few bugs in the online version of the tutorial - maybe the zip file/tarball at the end of the page has corrected code.

[dc740]

I just downloaded the tar version. and the code is the same.
I think it's a bug. The code won't break, but it's not the expected behaviour.

Maybe after a few replies (to be sure if this is a bug) we could ask for this correction to be added as a new page to the wiki.
Similar to:
http://wiki.osdev.org/Bran%27s_Known_Bugs

[pcmattman]

I believe the code should be:

Code: Select all

(align == 1) && (0x12345000 & 0xFFF)

Yes, that's a bug in the tutorial

[jal]

Anyone PMd James already?


JAL

[Creature]

I have asked him this question before and he has already noted it as a known bug in one of my older topics.

It is supposed to be:

Code: Select all

if(ShouldAlign && (Address & 0x00000FFF))
{
   Address &= 0xFFFFF000;
   Address += PAGE_SIZE;
}

Or:

Code: Select all

if(ShouldAlign && (Address % PAGE_SIZE))
{
   Address &= 0xFFFFF000;
   Address += PAGE_SIZE;
}

At least, that's if I recall correctly.

Finally, if you have an alignment that is variable, you could do something like:

Code: Select all

 if(alignment > 1)
{
   if(Address % alignment)
   {
      Address &= ~(alignment - 1);
      Address += alignment;
   }
}

[Gigasoft]

When aligning something, I prefer to write:
Something = ((Something - 1) | (Alignment - 1)) + 1

It makes the code shorter by eliminating the inclusion of the alignment constant twice.

[jal]

It makes the code shorter by eliminating the inclusion of the alignment constant twice.

True, but it obfuscates the intented meaning badly, and also mixes arithmetic operators with a binary one. Both are bad practice, imho.


JAL