On Wed, Jan 30, 2019 at 9:41 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
On Wed, Jan 30, 2019 at 9:32 AM Michael Schmitz <schmitzmic@xxxxxxxxx> 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.
At least for userspace, some people prefer avoiding the last page:
https://lore.kernel.org/lkml/CAG48ez0QRmU9fGV3x7nceGqBGADNTb66MnG4PW_xu+xnGUqg=g@xxxxxxxxxxxxxx/
m68knommu uses:
#define TASK_SIZE (0xFFFFFFFFUL)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
