improve grub_mm_init_region() debug message
PATCH 1/1 improve grub_mm_init_region() debug message
This has bothered me for a while. By changing %u to %p this debug message becomes actually useful for checking offset problems.
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call, if you are unable to speak? (as seen on /.)
* kern/mm.c (grub_mm_init_region): Improve debug message.
kern/mm.c +1 -1
@@ -143,7 +143,7 @@grub_mm_header_t h;grub_mm_region_t r, *p, q; -grub_dprintf ("mem" ,"Using memory for heap: addr=%p, size=%u\n" , addr, (unsigned int ) size); +grub_dprintf ("mem" ,"Using memory for heap: start=%p, end=%p\n" , addr, addr + size);/* If this region is too small, ignore it. */ if (size <GRUB_MM_ALIGN *2 )
Quoted 4 lines · Robert Millan
-
grub_dprintf("mem", "Using memory for heap: addr=%p, size=%u\n", addr, (unsigned int) size);
+grub_dprintf("mem", "Using memory for heap: start=%p, end=%p\n", addr, addr + size);
Maybe addr+size-1 would be better? Inclusive boundaries are more intuitive. Just run "cat /proc/iomem" to see what I mean.
Signature · 2 lines
Regards, Pavel Roskin
Quoted 9 lines · Pavel Roskin
Quoted 4 lines · Robert Millan
-
grub_dprintf("mem", "Using memory for heap: addr=%p, size=%u\n", addr, (unsigned int) size);
+grub_dprintf("mem", "Using memory for heap: start=%p, end=%p\n", addr, addr + size);Maybe addr+size-1 would be better? Inclusive boundaries are more intuitive. Just run "cat
/proc/iomem" to see what I mean.
Sounds better to me.
Does this change (with addr + size -1) sound fine to everyone else?
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call, if you are unable to speak? (as seen on /.)
Quoted 8 lines · Robert Millan
Quoted 7 lines · Pavel Roskin
Quoted 4 lines · Robert Millan
-
grub_dprintf("mem", "Using memory for heap: addr=%p, size=%u\n", addr, (unsigned int) size);
+grub_dprintf("mem", "Using memory for heap: start=%p, end=%p\n", addr, addr + size);
Quoted 6 lines
Quoted 2 lines
Maybe addr+size-1 would be better? Inclusive boundaries are more intuitive. Just run "cat
/proc/iomem" to see what I mean.Sounds better to me.
Does this change (with addr + size -1) sound fine to everyone else?
Uhm actually, since we already substract one on ieee1275 to workaround firmware bugs, this might get confusing.
How about just "addr + size" ?
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call… if you are unable to speak? (as seen on /.)
Quoted 12 lines · Robert Millan
Quoted 11 lines · Robert Millan
Quoted 9 lines · Pavel Roskin
Quoted 4 lines · Robert Millan
-
grub_dprintf("mem", "Using memory for heap: addr=%p, size=%u\n", addr, (unsigned int) size);
+grub_dprintf("mem", "Using memory for heap: start=%p, end=%p\n", addr, addr + size);Maybe addr+size-1 would be better? Inclusive boundaries are more intuitive. Just run "cat
/proc/iomem" to see what I mean.
Quoted 6 lines
Quoted 3 lines
Sounds better to me.
Does this change (with addr + size -1) sound fine to everyone else?
Uhm actually, since we already substract one on ieee1275 to workaround firmware bugs, this might get confusing.
Oh, I didn't know that.
Quoted 1 line
How about just "addr + size" ?
Fine with me.
Signature · 2 lines
Regards, Pavel Roskin
Quoted 8 lines · Robert Millan
Quoted 7 lines · Pavel Roskin
Quoted 4 lines · Robert Millan
-
grub_dprintf("mem", "Using memory for heap: addr=%p, size=%u\n", addr, (unsigned int) size);
+grub_dprintf("mem", "Using memory for heap: start=%p, end=%p\n", addr, addr + size);
Quoted 6 lines
Quoted 2 lines
Maybe addr+size-1 would be better? Inclusive boundaries are more intuitive. Just run "cat
/proc/iomem" to see what I mean.Sounds better to me.
Does this change (with addr + size -1) sound fine to everyone else?
Why not showing both the range and the size. Isn't the size useful sometimes? I hate calculating such things by hand...
--
Marco
Quoted 13 lines · Marco Gerards
Quoted 11 lines · Robert Millan
Quoted 9 lines · Pavel Roskin
Quoted 4 lines · Robert Millan
-
grub_dprintf("mem", "Using memory for heap: addr=%p, size=%u\n", addr, (unsigned int) size);
+grub_dprintf("mem", "Using memory for heap: start=%p, end=%p\n", addr, addr + size);Maybe addr+size-1 would be better? Inclusive boundaries are more intuitive. Just run "cat
/proc/iomem" to see what I mean.
Quoted 6 lines
Quoted 3 lines
Sounds better to me.
Does this change (with addr + size -1) sound fine to everyone else?
Why not showing both the range and the size. Isn't the size useful sometimes?
When? What you typicaly want to debug is whether your mem region overlaps with something else, or so.
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call… if you are unable to speak? (as seen on /.)
Quoted 16 lines · Robert Millan
Quoted 15 lines · Marco Gerards
Quoted 13 lines · Robert Millan
Quoted 9 lines · Pavel Roskin
Quoted 4 lines · Robert Millan
-
grub_dprintf("mem", "Using memory for heap: addr=%p, size=%u\n", addr, (unsigned int) size);
+grub_dprintf("mem", "Using memory for heap: start=%p, end=%p\n", addr, addr + size);Maybe addr+size-1 would be better? Inclusive boundaries are more intuitive. Just run "cat
/proc/iomem" to see what I mean.Sounds better to me.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Does this change (with addr + size -1) sound fine to everyone else?
Why not showing both the range and the size. Isn't the size useful sometimes?
When? What you typicaly want to debug is whether your mem region overlaps with something else, or so.
Well, it is fine for me. If I need the size, I will bring this up again. Feel free to commit the patch as suggested.
--
Marco
Quoted 3 lines · Marco Gerards
Well, it is fine for me. If I need the size, I will bring this up again. Feel free to commit the patch as suggested.
Ok then.
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call… if you are unable to speak? (as seen on /.)