Re: [RFC] Cachemap for 2.6.12rc4-mm1. Was Re: [PATCH] enhance x86 MTRR handling

[Dave Jones <davej@redhat.com>]

Hi Andi,

On Fri, May 13, 2005 at 03:29:45PM +0200, Andi Kleen wrote:

 > For memory (pfn_valid == 1) it would be more memory efficient to use a few bits
 > in struct page->flags

Maybe.

 > In general because there are lots of uses of "range lists" it would be better
 > to put it as a library into lib.

Ditto.

 > Coding style needs some work.

Yep.

 > Too many printks.

At least whilst this is getting polished, its worth keeping these
around. Once its stable, I agree, they can go (or at least be
demoted to dprintk's).   It seems that theres some problems with
the current code, so they're definitly useful..

for eg..

CMAP: cmap_request_range: 0xf8000000 - 0xf8100fff (1)
CMAP:     cachings mismatch (4 != 1)
CMAP: cmap_request_range: 0xf8101000 - 0xf8101fff (1)
CMAP:     cachings mismatch (4 != 1)
CMAP: cmap_request_range: 0xf8102000 - 0xf8301fff (1)
CMAP:     cachings mismatch (4 != 1)
[drm:radeon_do_init_cp] *ERROR* could not find ioremap agp regions!
CMAP: cmap_release_range: 0xff8f0000 - 0xff96efff
CMAP:    last user, freeing
CMAP:    last user, freeing
CMAP:    release_range successful!!

I'm not sure where that's coming from yet.  There's also a few
failures to release regions which need to be double checked.

 > I am not sure yet the cmaps don't need reference counting. For some
 > cases (user space support) they probably will.

Asides from cmap_entry->count ?
Hmm, there doesn't seem to be anything guarding concurrent accesses
to that.

 > Need user space support, e.g. using the existing ioctls for /proc/bus/pci/*
 > (they are currently not implemented on i386/x86-64 but should with this)
 > Then someone would need to change the X server to use this.

By hooking into ioremap(), we're getting this done automatically.

01:00.0 VGA compatible controller: ATI Technologies Inc Radeon RV200 QW [Radeon 7500] (prog-if 00 [VGA])
        Subsystem: PC Partner Limited RV200 QW [Radeon 7500 LE]
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping+ SERR+ FastB2B-
        Status: Cap+ 66Mhz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 32 (2000ns min), Cache Line Size 08
        Interrupt: pin A routed to IRQ 225
        Region 0: Memory at e8000000 (32-bit, prefetchable) [size=128M]
        Region 1: I/O ports at c800 [size=256]
        Region 2: Memory at ff8f0000 (32-bit, non-prefetchable) [size=64K]
        Expansion ROM at ff8c0000 [disabled] [size=128K]
        Capabilities: <available only to root>

(10:39:17:davej@dhcp83-2:~)$ grep e8000000 /proc/cachemap
 0xe8000000 - 0xebffffff: 0x0004 1

Though I agree a userspace interface could be useful.

 > Need to figure out if CMAP_ALLOW_OVERLAPPING should be set or not.

*nod*, and if it should, lose the ifdef completely.
 
 > Probably need to go over the combining rule etc. with a fine comb

agreed.  There's a bunch of errata on older CPUs that should probably
be checked too.

Thanks for the comments.  I'm not working on this full-time, but
I'll continue to poke at it occasionally, especially if theres
interest from anyone else.

		Dave