Re: Bug: Discontigmem virt_to_page() [Alpha,ARM,Mips64?]

[Daniel Phillips <phillips@bonn-fries.net>]

On Monday 06 May 2002 21:07, Roman Zippel wrote:
> Hi,
> 
> On Mon, 6 May 2002, Daniel Phillips wrote:
> 
> > I don't, I observed that in all known instances of config_discontigmem, 
> > that linear relationship is preserved.
> 
> That's true, but m68k isn't using config_discontigmem. :)

Right.  In fact, your two way, phys_to_virt and virt_to_phys mapping makes it 
more like config_nonlinear.  You don't define the contiguous logical memory 
space though, and perhaps that's the reason you need the free_area_init 
changes in the patch below.

Your patch preserves a linear relationship between physical and virtual 
memory, because you do both the ptov and vtop lookup in the same array.  As
such, you don't provide the functionality I provide of being able to fit a
large amount of physical memory into a small amount of virtual memory, and
you can't join all your separate pgdat's into one, as I do.  (The latter is 
desireable because it allows the memory manager to allocate from one 
homogeneous space, reducing the likelihood of zone balancing problems.)

We could, if we want, implement your variable sized memory chunk system with
config_nonlinear. You'd just have to replace the

    ulong psection[MAX_SECTIONS]

with:

    struct { ulong base; ulong size; } pchunk[MAX_CHUNKS];

and replace the four direct table lookup with loops.  Highmem does not need
to be a special case, by the way.  Another by the way: you've accidently
repeated the last four lines of mm_vtop.  Finally, it looks like your 
ZTWO_VADDR hack in mm_ptov would also cease to be a special case, at least,
the special case part would move to the initialization instead of every __va
operation.  So you would end up with *zero* special cases in the page 
translation functions of page.h.

> ...you only have to take care, that you don't iterate
> with the physical address over a pgdat, this is what the patch below
> fixes, the rest can be hidden in the arch macros and no special config
> options is needed.

You do have a config option, it's CONFIG_SINGLE_MEMORY_CHUNK.  You just didn't
attempt to create the contiguous logical address space as I did, so you 
didn't need to go outside your arch.

The generic part of config_nonlinear is tiny anyway - only 200 lines, and 
might grow to 400 by the time all device driver usage of __pa is reclassified 
as either virt_to_phys or virt_to_logical - the latter being a rather nice 
distinction to make, even if the mapping is the same don't you think?  I.e, 
it's like the distinction between pointer and integer: if it's a physical 
address you can pass it to dma hardware, for example and if it's logical 
you're just using it for accounting.

Whenever it's possible to elevate a per-arch feature to the generic level
without compromising functionality, it should be done, modulo programmer 
time, and of course, assuming functionality isn't compromised.  At 
the generic level, it's easier to document, we get cross-pollination from 
improvements developed on different arches, and it's easier to build on.  
Going the other way and allowing design features to fray across architectures 
takes us in the direction of unmaintainable bloat.

-- 
Daniel