[parisc-linux] 2.4.0-test6 lack of speed

[parisc-linux] 2.4.0-test6 lack of speed

[Richard Hirst]

I put some printk's in pgalloc.h:

__flush_dcache_range: addr 0xc7e23000, size 4096
__flush_icache_range: addr 0xc7e23000, size 4096
__flush_icache_range: addr 0xc7e23000, size 4096
__flush_dcache_range: addr 0xc7da5000, size 4096
__flush_icache_range: addr 0xc7da5000, size 4096
__flush_dcache_range: addr 0x000bdc8c, size 4096
__flush_icache_range: addr 0x000bdc8c, size 4096
__flush_dcache_range: addr 0x40000000, size 4096
__flush_icache_range: addr 0x40000000, size 4096
__flush_dcache_range: addr 0xc7dc8000, size 4096
__flush_icache_range: addr 0xc7dc8000, size 4096
__flush_dcache_range: addr 0xc7dc8000, size 4096
__flush_icache_range: addr 0xc7dc8000, size 4096
__flush_icache_range: addr 0xc7dc8000, size 4096
__flush_dcache_range: addr 0x00001000, size 770048
__flush_icache_range: addr 0x00001000, size 770048
__flush_dcache_range: addr 0x000bd000, size 24576
__flush_icache_range: addr 0x000bd000, size 24576
__flush_dcache_range: addr 0x000c3000, size 102400
__flush_icache_range: addr 0x000c3000, size 102400
__flush_dcache_range: addr 0x2001f000, size 268308480
__flush_icache_range: addr 0x2001f000, size 268308480
__flush_dcache_range: addr 0x40100000, size 32768
__flush_icache_range: addr 0x40100000, size 32768
__flush_dcache_range: addr 0x40108000, size 1015808
__flush_icache_range: addr 0x40108000, size 1015808
__flush_dcache_range: addr 0x20022fac, size 4096
__flush_icache_range: addr 0x20022fac, size 4096
__flush_dcache_range: addr 0x00003594, size 2
__flush_dcache_range: addr 0xc7f986c8, size 2
> __flush_dcache_range: addr 0x2002300c, size 4096
__flush_icache_range: addr 0x2002300c, size 4096


Those two flushes of 268 Meg each take a while to run.
There also appears to be a mix of virtual and physical addresses being
passed to __flush_[id]cache_range(), which must be wrong.  I believe they
should be passed virtual addresses.

pgtable.h is interesting as well:

/*
 * cache_push() semantics: Write back any dirty cache data in the given area,
 * and invalidate the range in the instruction cache. It needs not (but may)
 * invalidate those entries also in the data cache. The range is defined by a
 * _physical_ address.
 */
#define cache_push(paddr, len) \
                        flush_cache_range(&init_mm, virt_to_phys(paddr), len)
#define cache_push_v(vaddr, len) \
                        flush_cache_range(&init_mm, vaddr, len)


I guess that virt_to_phys() should be a phys_to_virt()....


Richard

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Richard Hirst]

On Tue, Aug 22, 2000 at 03:38:03PM +0100, Richard Hirst wrote:
> I put some printk's in pgalloc.h:
> 
> __flush_dcache_range: addr 0xc7e23000, size 4096
> __flush_icache_range: addr 0xc7e23000, size 4096
> __flush_icache_range: addr 0xc7e23000, size 4096
> __flush_dcache_range: addr 0xc7da5000, size 4096
> __flush_icache_range: addr 0xc7da5000, size 4096
> __flush_dcache_range: addr 0x000bdc8c, size 4096
> __flush_icache_range: addr 0x000bdc8c, size 4096
> __flush_dcache_range: addr 0x40000000, size 4096
> __flush_icache_range: addr 0x40000000, size 4096
> __flush_dcache_range: addr 0xc7dc8000, size 4096
> __flush_icache_range: addr 0xc7dc8000, size 4096
> __flush_dcache_range: addr 0xc7dc8000, size 4096
> __flush_icache_range: addr 0xc7dc8000, size 4096
> __flush_icache_range: addr 0xc7dc8000, size 4096
> __flush_dcache_range: addr 0x00001000, size 770048
> __flush_icache_range: addr 0x00001000, size 770048
> __flush_dcache_range: addr 0x000bd000, size 24576
> __flush_icache_range: addr 0x000bd000, size 24576
> __flush_dcache_range: addr 0x000c3000, size 102400
> __flush_icache_range: addr 0x000c3000, size 102400
> __flush_dcache_range: addr 0x2001f000, size 268308480
> __flush_icache_range: addr 0x2001f000, size 268308480
> __flush_dcache_range: addr 0x40100000, size 32768
> __flush_icache_range: addr 0x40100000, size 32768
> __flush_dcache_range: addr 0x40108000, size 1015808
> __flush_icache_range: addr 0x40108000, size 1015808
> __flush_dcache_range: addr 0x20022fac, size 4096
> __flush_icache_range: addr 0x20022fac, size 4096
> __flush_dcache_range: addr 0x00003594, size 2
> __flush_dcache_range: addr 0xc7f986c8, size 2
> > __flush_dcache_range: addr 0x2002300c, size 4096
> __flush_icache_range: addr 0x2002300c, size 4096
> 
> 
> Those two flushes of 268 Meg each take a while to run.
> There also appears to be a mix of virtual and physical addresses being
> passed to __flush_[id]cache_range(), which must be wrong.  I believe they
> should be passed virtual addresses.
> 
> pgtable.h is interesting as well:
> 
> /*
>  * cache_push() semantics: Write back any dirty cache data in the given area,
>  * and invalidate the range in the instruction cache. It needs not (but may)
>  * invalidate those entries also in the data cache. The range is defined by a
>  * _physical_ address.
>  */
> #define cache_push(paddr, len) \
>                         flush_cache_range(&init_mm, virt_to_phys(paddr), len)
> #define cache_push_v(vaddr, len) \
>                         flush_cache_range(&init_mm, vaddr, len)
> 
> 
> I guess that virt_to_phys() should be a phys_to_virt()....

And also, flush_cache_range() is defined in pgalloc.h as taking start and
end, not start and length.

Richard

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Richard Hirst]

On Tue, Aug 22, 2000 at 03:52:21PM +0100, Richard Hirst wrote:
> > __flush_dcache_range: addr 0x00001000, size 770048
> > __flush_icache_range: addr 0x00001000, size 770048
> > __flush_dcache_range: addr 0x000bd000, size 24576
> > __flush_icache_range: addr 0x000bd000, size 24576
> > __flush_dcache_range: addr 0x000c3000, size 102400
> > __flush_icache_range: addr 0x000c3000, size 102400
> > __flush_dcache_range: addr 0x2001f000, size 268308480
> > __flush_icache_range: addr 0x2001f000, size 268308480

Those ones are caused by a call to flush_page_range() being added to
mm/mmap.c:exit_mmap().

We just blindly assume addresses passed to flush_page_range() are
kernel virtual addresses, but in this case I guess they are user
process virtual addresses.  Even so, the last one below looks
like a rather large area to have mmapped.

exit_mmap: calling flush_cache_range(0x00001000, 0x00083000)
exit_mmap: calling flush_cache_range(0x00083000, 0x00085000)
exit_mmap: calling flush_cache_range(0x00085000, 0x00088000)
exit_mmap: calling flush_cache_range(0x2001f000, 0x30000000)

(the exit_mmap debug is from a different kernel build, so don't
try and match the numbers with those quoted above)

Richard

Re: [parisc-linux] 2.4.0-test6 lack of speed

[David Huggins-Daines]

Richard Hirst <rhirst@linuxcare.com> writes:

> We just blindly assume addresses passed to flush_page_range() are
> kernel virtual addresses, but in this case I guess they are user
> process virtual addresses.

Right.  I noticed that the FIC/FICE/FDC/FDCE instructions have a space
register field.  I wonder if we should be explicitly specifying %sr3,
since that's what we use (ahem, what we *WOULD* use if <asm/uaccess.h>
were actually implemented) to access user space.

Are PA-RISC caches indexed with the space ID as well as the offset?
Do we need to flush kernel virtual addresses at all?

> exit_mmap: calling flush_cache_range(0x00001000, 0x00083000)

text

> exit_mmap: calling flush_cache_range(0x00083000, 0x00085000)

data and bss

> exit_mmap: calling flush_cache_range(0x00085000, 0x00088000)

heap

> exit_mmap: calling flush_cache_range(0x2001f000, 0x30000000)

stack

>  Even so, the last one below looks like a rather large area to have
> mmapped.

Yes, it's arbitrarily huge, and this is a VM problem that needs to be
fixed.  It should grow upwards as needed via the page fault handler.
Paging John Marvin... (pun intended ;-)

-- 
dhd@linuxcare.com, http://www.linuxcare.com/
Linuxcare. Support for the revolution.

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Richard Hirst]

On Tue, Aug 22, 2000 at 12:05:06PM -0400, David Huggins-Daines wrote:
> Richard Hirst <rhirst@linuxcare.com> writes:
> 
> > We just blindly assume addresses passed to flush_page_range() are
> > kernel virtual addresses, but in this case I guess they are user
> > process virtual addresses.
> 
> Right.  I noticed that the FIC/FICE/FDC/FDCE instructions have a space
> register field.  I wonder if we should be explicitly specifying %sr3,
> since that's what we use (ahem, what we *WOULD* use if <asm/uaccess.h>
> were actually implemented) to access user space.
> 
> Are PA-RISC caches indexed with the space ID as well as the offset?
> Do we need to flush kernel virtual addresses at all?

I should have said flush_cache_range() above, not flush_page_range().

It looks to me like all calls to flush_cache_range() are passed a user
virtual address, except for those calls from arch/parisc/kernel/pci-dma.c
where we pass kernel virtual addresses.

So perhaps flush_cache_range() should generate FIC/FICE/FDC/FDCE
instructions with a space register of %sr3, and pci-dma.c should
use some other method of flushing memory.

I was looking at the difference between FDC and FDCE; it seems FDC
does address translation and FDCE does not, so should we use FDC
really?  flush_cache_range() currently uses FDCE/FICE.

Helge Deller wrote some code in drivers/net/lasi_82596.c that plays
with %sr1 and uses FDC, FIC, PDC, SYNC, and SYNCDMA.

Can someone tell me why include/asm/cache.h has no space register
specified on fdce():

> #define fdce(addr) asm volatile("fdce 0(%0)" : : "r" (addr))
> #define fice(addr) asm volatile("fice 0(%%sr1,%0)" : : "r" (addr))
> 
> #define pdtlbe(addr) asm volatile("pdtlbe 0(%%sr1,%0)" : : "r" (addr))
> #define pitlbe(addr) asm volatile("pitlbe 0(%%sr1,%0)" : : "r" (addr))


Richard

ps. as you can see, I am learning as I go here, so all hints and
clues are gratefully accepted!

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Paul Bame]

jsm explained to me that the F?CE and P?TLBE instructions should
only be used "for flushing the entire data or combined cache".
The operands of F?CE/P?TLBE are "an implementation-dependent function
of the effective address", essentially an opaque data type that
varies per processor.  The operand of F?C/P?TLB is a virtual address,
so is the right thing to use for flushing a range of addresses.

	-P

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Richard Hirst]

On Wed, Aug 23, 2000 at 10:38:13AM -0600, Paul Bame wrote:
> 
> jsm explained to me that the F?CE and P?TLBE instructions should
> only be used "for flushing the entire data or combined cache".
> The operands of F?CE/P?TLBE are "an implementation-dependent function
> of the effective address", essentially an opaque data type that
> varies per processor.  The operand of F?C/P?TLB is a virtual address,
> so is the right thing to use for flushing a range of addresses.

OK, thanks.  We use F?CE/P?TLBE in cache.h atm.

Richard

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Matthew Wilcox]

On Wed, Aug 23, 2000 at 05:23:20PM +0100, Richard Hirst wrote:
> It looks to me like all calls to flush_cache_range() are passed a user
> virtual address, except for those calls from arch/parisc/kernel/pci-dma.c
> where we pass kernel virtual addresses.

agreed.

> So perhaps flush_cache_range() should generate FIC/FICE/FDC/FDCE
> instructions with a space register of %sr3, and pci-dma.c should
> use some other method of flushing memory.
> 
> I was looking at the difference between FDC and FDCE; it seems FDC
> does address translation and FDCE does not, so should we use FDC
> really?  flush_cache_range() currently uses FDCE/FICE.

we should stop using FICE/FDCE/PDTLBE/PITLBE except in the `flush
entire cache' routines.  the definitions should be moved from cache.h
to cache.c.  No-one else should be given the opportunaity to use them.
They are too undefined.

> Helge Deller wrote some code in drivers/net/lasi_82596.c that plays
> with %sr1 and uses FDC, FIC, PDC, SYNC, and SYNCDMA.
> 
> Can someone tell me why include/asm/cache.h has no space register
> specified on fdce():
> 
> > #define fdce(addr) asm volatile("fdce 0(%0)" : : "r" (addr))
> > #define fice(addr) asm volatile("fice 0(%%sr1,%0)" : : "r" (addr))
> > 
> > #define pdtlbe(addr) asm volatile("pdtlbe 0(%%sr1,%0)" : : "r" (addr))
> > #define pitlbe(addr) asm volatile("pitlbe 0(%%sr1,%0)" : : "r" (addr))

My attempt at fixing this in a sane manner led me to create:

#define user_fdc(addr) asm volatile("fdc 0(%%sr3,%0)" : : "r" (addr))
(...)

and

#define kernel_fdc(addr) asm volatile("fdc 0(%%sr0,%0)" : : "r" (addr))
(...)

however, the resulting kernel wouldn't boot.  So let's take this a
step at a time and see how far we can go towards eliminating the *E
variants.  I've written a little rant on this suject which you can find
in linux-2.3/Documentation/parisc/mm.  feel free to ignore it if you want.

> ps. as you can see, I am learning as I go here, so all hints and
> clues are gratefully accepted!

Likewise!  BTW, I hear davem wrote a doc about this which is in
-test8-pre1 which may help somewhat.

-- 
Revolutions do not require corporate support.

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Matthew Wilcox]

On Tue, Aug 29, 2000 at 05:23:08PM -0400, Matthew Wilcox wrote:
> > Helge Deller wrote some code in drivers/net/lasi_82596.c that plays
> > with %sr1 and uses FDC, FIC, PDC, SYNC, and SYNCDMA.

> #define kernel_fdc(addr) asm volatile("fdc 0(%%sr0,%0)" : : "r" (addr))
> 
> however, the resulting kernel wouldn't boot.  So let's take this a
> step at a time and see how far we can go towards eliminating the *E
> variants.  I've written a little rant on this suject which you can find
> in linux-2.3/Documentation/parisc/mm.  feel free to ignore it if you want.

ok.  we now have interfaces for flushing kernel dcache.  I'm assuming
that no-one is intending to dma to/from an instruction page, so i
haven't included an icache variant.  i'm not using sync or syncdma yet.
perhaps i should be.  are we out-of-order right now or have we told the
MMU to strongly order our writes?

Anyway, would either of Richard or Helge like to look over the lasi_82596
driver and convert it to use the flush_kernel_dcache_range() interface
or tell me why my code is no good and I suck?  Ditto the sim700 driver
actually.  Are there any other drivers which have simply taken the
lasi_82596 wback code and replicated it or are these the only two?

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Richard Hirst]

On Wed, Aug 30, 2000 at 01:17:52AM +0100, Matthew Wilcox wrote:
> Anyway, would either of Richard or Helge like to look over the lasi_82596
> driver and convert it to use the flush_kernel_dcache_range() interface
> or tell me why my code is no good and I suck?  Ditto the sim700 driver
> actually.  Are there any other drivers which have simply taken the
> lasi_82596 wback code and replicated it or are these the only two?

Only those two, so far as I'm aware.  I'll have a look at them.

Richard

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Richard Hirst]

On Tue, Aug 29, 2000 at 05:23:08PM -0400, Matthew Wilcox wrote:
> My attempt at fixing this in a sane manner led me to create:
> 
> #define user_fdc(addr) asm volatile("fdc 0(%%sr3,%0)" : : "r" (addr))
> (...)
> 
> and
> 
> #define kernel_fdc(addr) asm volatile("fdc 0(%%sr0,%0)" : : "r" (addr))
> (...)
> 
> however, the resulting kernel wouldn't boot.  So let's take this a

I tried something similar, with the same result.

What do you make of arch/parisc/lib/usercopy.c

unsigned long
__generic_copy_to_user(void *to, const void *from, unsigned long n)
{
        if (access_ok(VERIFY_WRITE, to, n)) {
                __flush_dcache_range((unsigned long)from, n);
                lcopy_to_user(to,from,n);
                __flush_dcache_range((unsigned long)to, n);
        }
        return 0;
}


Typically 'from' would be a kernel virtual address, and 'to' would
be a user virtual address, yes?  So, that is expecting __flush_dcache_range()
to know which space register to use.  But then sometimes these functions
for accessing user space are redirected at kernel space by set_fs().
In that case both addresses are kernel virtual addresses.

lcopy_to_user() claims to modify %sr1 to get the right space in %sr1,
based on a flag in the task struct.

See copy_strings_kernel() in fs/exec.c for an example of where
copy_from_user( is used to copy from kernel space.

Richard

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Richard Hirst]

On Wed, Aug 30, 2000 at 01:17:52AM +0100, Matthew Wilcox wrote:
> Anyway, would either of Richard or Helge like to look over the lasi_82596
> driver and convert it to use the flush_kernel_dcache_range() interface
> or tell me why my code is no good and I suck?  Ditto the sim700 driver
> actually.  Are there any other drivers which have simply taken the
> lasi_82596 wback code and replicated it or are these the only two?

I'm not that happy with calling flush_kernel_dcache_range() from
drivers, when that function only exists on parisc.  Most archs define
dma_cache_wback/dma_cache_inv/dma_cache_wback_inv, which, from the
descriptions in asm-ia64/io.h seem to be just what lasi_82596 and
sim700 need.  The versions on parisc currently just flush_all_caches(),
but perhaps we should make all three call flush_kernel_dcache_range()?
I'll try that approach anyway.

Richard

Re: [parisc-linux] 2.4.0-test6 lack of speed

[Matthew Wilcox]

On Wed, Aug 30, 2000 at 11:15:23AM +0100, Richard Hirst wrote:
> What do you make of arch/parisc/lib/usercopy.c
> 
> unsigned long
> __generic_copy_to_user(void *to, const void *from, unsigned long n)
> {
>         if (access_ok(VERIFY_WRITE, to, n)) {
>                 __flush_dcache_range((unsigned long)from, n);
>                 lcopy_to_user(to,from,n);
>                 __flush_dcache_range((unsigned long)to, n);
>         }
>         return 0;
> }
> 
> Typically 'from' would be a kernel virtual address, and 'to' would
> be a user virtual address, yes?  So, that is expecting __flush_dcache_range()
> to know which space register to use.  But then sometimes these functions
> for accessing user space are redirected at kernel space by set_fs().
> In that case both addresses are kernel virtual addresses.

Right.  But I don't see why we need to flush any caches at all here.
We're not changing any mappings, we're accessing through a fixed space
register, there's no other device involved, if there were another CPU
involved then it would be cache-coherent _anyway_ -- what's the problem?