Hi Michael,
On Wed, Jan 30, 2019 at 9:32 AM Michael Schmitz <schmitz...@gmail.com> wrote:
> Am 30.01.2019 um 21:08 schrieb Geert Uytterhoeven:
> >> The remainder is a fix for address wrap around when there is memory located
> >> at the end of the 32-bit address space.
> >> That part looks OK to me, and is still applicable.
> >>
> >> I will retest with just the last part of the patch applied.
> >>
> >> max_addr was reduced by one earlier in the last part, so isn't this one
> >>
> >> min_low_pfn = availmem >> PAGE_SHIFT;
> >> - max_low_pfn = max_addr >> PAGE_SHIFT;
> >> + max_low_pfn = (max_addr >> PAGE_SHIFT) + 1;
> >>
> >> wrong? i.e. shouldn't we use
> >>
> >> max_low_pfn = (max_addr+1) >> PAGE_SHIFT;
> >>
> >> instead?
> >
> > That would give 0 if max_addr = 0xffffffff, due to overflow.
>
> Yep, figured that out right after hitting 'send'. The PFN doesn't
> overflow, but wouldn't calculating end addresses from max_low_pfn lead
> to overflow later on? It seems to me that max_low_pfn would point to the
> page right after max_addr i.e. beyond end of memory? Is that expected
> behaviour?
Yes, people shifting max_low_pfn or max_pfn by PAGE_SIZE may get back
zero, if doing the shift in 32-bit arithmetic.
The m68k-specific code doesn't seem to do that, but who knows about
generic (PC-centric? ;-) code...
I see microblaze does:
max_low_pfn = ((u64)memory_start + (u64)lowmem_size) >> PAGE_SHIFT;
so probably they also encountered memory at the end of the address space.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
