flush_kernel_dcache_page() not needed ?
Hi,
I noticed that there's currently (v2.6.23-rc4) no implementation of this function even for mips CPUs that have dcache aliasing.
Could anybody explain why ?
thanks,
Franck
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Ralf added this a while ago:
"arch/mips/mm/cache.c:void __flush_dcache_page(struct page *page)"
//Markus
Quoted 9 lines · Franck Bui-Huu
Hi,
I noticed that there's currently (v2.6.23-rc4) no implementation of this function even for mips CPUs that have dcache aliasing.
Could anybody explain why ?
thanks, Franck
- --
_______________________________________
Mr Markus Gothe
Software Engineer
Phone: +46 (0)13 21 81 20 (ext. 1046)
Fax: +46 (0)13 21 21 15
Mobile: +46 (0)73 718 72 80 Diskettgatan 11, SE-583 35 Linköping, Sweden www.27m.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFG2Cqe6I0XmJx2NrwRCNouAKDTex/fiBt4qJgX15tW4TzwQct7DQCgi6sE GNFJZe/vjsW3z/FQw01yxx0=
=RFVE
-----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
N/M, I was thinking of flush_Anon_Page
Quoted 9 lines · Franck Bui-Huu
Hi,
I noticed that there's currently (v2.6.23-rc4) no implementation of this function even for mips CPUs that have dcache aliasing.
Could anybody explain why ?
thanks, Franck
- --
_______________________________________
Mr Markus Gothe
Software Engineer
Phone: +46 (0)13 21 81 20 (ext. 1046)
Fax: +46 (0)13 21 21 15
Mobile: +46 (0)73 718 72 80 Diskettgatan 11, SE-583 35 Linköping, Sweden www.27m.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFG2CuW6I0XmJx2NrwRCOXHAJ4gs+pw2Ra3X27MFgpjx9V3B7WbSACgviej 7K8mydrGQPHKZWikCsGopVM=
=nOcy
-----END PGP SIGNATURE-----
Hi,
Quoted 3 lines · Markus Gothe
Ralf added this a while ago:
"arch/mips/mm/cache.c:void
__flush_dcache_page(struct page *page)"
I was asking for flush_kernel_dcache_page()...
Please note the word "kernel" in the name of the function.
Franck
Quoted 3 lines · Markus Gothe
Ralf added this a while ago:
"arch/mips/mm/cache.c:void
__flush_dcache_page(struct page *page)"
__flush_dcache_page is just the part of flush_dcache_page which does the real work.and has nothing to do with flush_kernel_dcache_page.
Ralf
I thought I mailed out 'N/M' shortly after the first mail ;)
Quoted 10 lines · Ralf Baechle
Quoted 3 lines · Markus Gothe
Ralf added this a while ago:
"arch/mips/mm/cache.c:void
__flush_dcache_page(struct page *page)"
__flush_dcache_pageis just the part offlush_dcache_pagewhich does the real work.and has nothing to do withflush_kernel_dcache_page.Ralf
_______________________________________
Mr Markus Gothe
Software Engineer
Phone: +46 (0)13 21 81 20 (ext. 1046)
Fax: +46 (0)13 21 21 15
Mobile: +46 (0)73 718 72 80 Diskettgatan 11, SE-583 35 Linköping, Sweden www.27m.com
Quoted 4 lines · Franck Bui-Huu
I noticed that there's currently (v2.6.23-rc4) no implementation of this function even for mips CPUs that have dcache aliasing.
Could anybody explain why ?
Maybe because the API was not called by anybody until 2.6.23-rc1 :)
Now copy_strings() calls this and I'm wondering we should implement or not. It seems the kernel works fine for me without the API...
Do you have any problem due to luck of the API?
---
Atsushi Nemoto
Quoted 8 lines · Atsushi Nemoto
Quoted 4 lines · Franck Bui-Huu
I noticed that there's currently (v2.6.23-rc4) no implementation of this function even for mips CPUs that have dcache aliasing.
Could anybody explain why ?
Maybe because the API was not called by anybody until 2.6.23-rc1 :)
But this function has been introduced since commit 5a3a5a98b6422d05c39eaa32c8b3f83840c7b768 ([PATCH] Add flush_kernel_dcache_page() API) which had been merged during 2.6.16 merge window. So it's more than one year now...
Basically it gives a default implementation for all architectures. The problem here is that this implementation may be boggus for architectures that have dcache aliasing issue.
The sad thing is that the kernel will silently compile this default implementation. At least, it could have showed a big fat warning during the building process.
Quoted 2 lines
Now
copy_strings()calls this and I'm wondering we should implement or not. It seems the kernel works fine for me without the API...
Do you use a cpu with dcache aliasing issue ?
Quoted 1 line
Do you have any problem due to luck of the API?
No, but looking at copy_strings(), I think we can have some trouble.
BTW, do you recall flush_anon_page() and fuse bug ? It seems the same here...
Franck
Quoted 4 lines · Franck Bui-Huu
Quoted 2 lines
Now
copy_strings()calls this and I'm wondering we should implement or not. It seems the kernel works fine for me without the API...Do you use a cpu with dcache aliasing issue ?
Yes. 4 way 32KB dcache.
Quoted 6 lines
Quoted 1 line
Do you have any problem due to luck of the API?
No, but looking at
copy_strings(), I think we can have some trouble.BTW, do you recall
flush_anon_page()and fuse bug ? It seems the same here...
Indeed. But copy_strings() is not rare case (called on each execve), so there might be some constraints which make us free from the aliasing problem. I'll look at it further, but any testcase are welcome.
---
Atsushi Nemoto
Quoted 3 lines · Atsushi Nemoto
Indeed. But
copy_strings()is not rare case (called on each execve), so there might be some constraints which make us free from the aliasing problem.
The point is that this _generic_ function has been created so we need to understand if MIPS architecture needs to implement it or not, whatever its current usages. This was actually what I was trying to understand with this thread.
Whatever the constraints, they don't seem to be intended at all since flush_kernel_dcache_page() is called... So even if the current code is working fine, it seems very fragile _if_ MIPS needs to implement this cache helper.
Quoted 1 line
I'll look at it further, but any testcase are welcome.
One thing you might want to try is:
$ echo 0 > /proc/sys/kernel/randomize_va_space
and see if your system still works fine. This command should avoid a data cache flush when moving the stack around. See shift_arg_pages().
With this, maybe you could give this testcase a try:
$ /bin/echo "`seq 10000`" > seq.txt
and see if seq.txt is correct. This command should pass to echo (not the bash builtin one) a long argument that should fill your dcache.
That said the execve syscall code is quite 'hairy' and it may not be suprising that after this syscall the dcache has been completly flushed and thus make the problem disappear.
Franck
Quoted 9 lines · Franck Bui-Huu
Quoted 3 lines
Indeed. But
copy_strings()is not rare case (called on each execve), so there might be some constraints which make us free from the aliasing problem.The point is that this _generic_ function has been created so we need to understand if MIPS architecture needs to implement it or not, whatever its current usages. This was actually what I was trying to understand with this thread.
Quoted 4 lines
Whatever the constraints, they don't seem to be intended at all since
flush_kernel_dcache_page()is called... So even if the current code is working fine, it seems very fragile _if_ MIPS needs to implement this cache helper.
Yes, agreed.
Quoted 1 line
Quoted 1 line
I'll look at it further, but any testcase are welcome.
I found the reason.
do_execve()
copy_strings()
flush_kernel_dcache_page()
search_binary_handler()
load_elf_binary()
setup_arg_pages()
shift_arg_pages()
move_page_tables()
flush_cache_range()
And MIPS flush_cache_range() blasts whole dcache! That's why empty flush_kernel_dcache_page() was enough for now.
Anyway, here is a patch.
Subject: [MIPS] Implement flush_kernel_dcache_page()
---
include/asm-mips/cacheflush.h +7 -0
@@ -79,6 +79,13 @@ extern void (*flush_icache_all)(void);extern void (*local_flush_data_cache_page)(void * addr);extern void (*flush_data_cache_page)(unsigned long addr); +#define ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE +static inline void flush_kernel_dcache_page (struct page *page) +{ +if (cpu_has_dc_aliases) +flush_data_cache_page ((unsigned long )page_address (page)); +} +/* * This flag is used to indicate that the page pointed to by a pte * is dirty and requires cleaning before returning it to the user.
Quoted 15 lines · Atsushi Nemoto
Quoted 1 line · Franck Bui-Huu
Quoted 1 line
I'll look at it further, but any testcase are welcome.
I found the reason.
do_execve()
copy_strings()
flush_kernel_dcache_page()
search_binary_handler()
load_elf_binary()
setup_arg_pages()
shift_arg_pages()
move_page_tables()
flush_cache_range()
Quoted 2 lines
And MIPS
flush_cache_range()blasts whole dcache! That's why emptyflush_kernel_dcache_page()was enough for now.
Yeah, that what I think too and that's also why I was suggesting to test a plain 2.6.23-rc5 with the randomize_va_space=0. Please see my previous reply...
Could you give it a try ?
Franck
Quoted 5 lines · Franck Bui-Huu
Yeah, that what I think too and that's also why I was suggesting to test a plain 2.6.23-rc5 with the randomize_va_space=0. Please see my previous reply...
Could you give it a try ?
OK, I'll try it in a few days...
---
Atsushi Nemoto
Quoted 8 lines · Atsushi Nemoto
Quoted 5 lines · Franck Bui-Huu
Yeah, that what I think too and that's also why I was suggesting to test a plain 2.6.23-rc5 with the randomize_va_space=0. Please see my previous reply...
Could you give it a try ?
OK, I'll try it in a few days...
thank you, Atsushi.
Franck
Quoted 7 lines · Franck Bui-Huu
One thing you might want to try is:
$ echo 0 > /proc/sys/kernel/randomize_va_spaceand see if your system still works fine. This command should avoid a data cache flush when moving the stack around. See
shift_arg_pages().
Quoted 7 lines
With this, maybe you could give this testcase a try:
$ /bin/echo "`seq 10000`" > seq.txtand see if seq.txt is correct. This command should pass to echo (not the bash builtin one) a long argument that should fill your dcache.
I tried this and everything worked fine with/without the flush_kernel_dcache_page() ;)
Quoted 3 lines
That said the execve syscall code is quite 'hairy' and it may not be suprising that after this syscall the dcache has been completly flushed and thus make the problem disappear.
Yes, there is an yet another path to "flush all dcache".
do_execve()
copy_strings()
flush_kernel_dcache_page()
search_binary_handler()
load_elf_binary()
flush_old_exec()
exec_mmap()
mmput()
exit_mmap()
flush_cache_mm()
r4k_blast_dcache()Anyway, the implementation of flush_kernel_dcache_page() is very simple so that we can believe it works correctly without any testcase. Too optimistic? :)
---
Atsushi Nemoto
Quoted 37 lines · Atsushi Nemoto
Quoted 7 lines · Franck Bui-Huu
One thing you might want to try is:
$ echo 0 > /proc/sys/kernel/randomize_va_spaceand see if your system still works fine. This command should avoid a data cache flush when moving the stack around. See
shift_arg_pages().Quoted 7 lines
With this, maybe you could give this testcase a try:
$ /bin/echo "`seq 10000`" > seq.txtand see if seq.txt is correct. This command should pass to echo (not the bash builtin one) a long argument that should fill your dcache.
I tried this and everything worked fine with/without the
flush_kernel_dcache_page();)Quoted 3 lines
That said the execve syscall code is quite 'hairy' and it may not be suprising that after this syscall the dcache has been completly flushed and thus make the problem disappear.
Yes, there is an yet another path to "flush all dcache".
do_execve()
copy_strings()
flush_kernel_dcache_page()
search_binary_handler()load_elf_binary() flush_old_exec() exec_mmap() mmput() exit_mmap() flush_cache_mm() r4k_blast_dcache()
Quoted 3 lines
Anyway, the implementation of
flush_kernel_dcache_page()is very simple so that we can believe it works correctly without any testcase. Too optimistic? :)
god only knows ;)
Franck
Quoted 6 lines · Atsushi Nemoto
Yes, there is an yet another path to "flush all dcache".
do_execve()
copy_strings()
flush_kernel_dcache_page()
search_binary_handler()
Quoted 8 lines
load_elf_binary()
flush_old_exec()
exec_mmap()
mmput()
exit_mmap()
flush_cache_mm()
r4k_blast_dcache()BTW, flush_cache_mm() flushes (write back + invalidate ) the whole data cache unconditionnaly, but I'm wondering if it's really necessary for cpus which don't have any cache aliasing issues. After all they're equivalent to physical caches, aren't they ?
thanks,
Franck
Quoted 4 lines · Franck Bui-Huu
BTW,
flush_cache_mm()flushes (write back + invalidate ) the whole data cache unconditionnaly, but I'm wondering if it's really necessary for cpus which don't have any cache aliasing issues. After all they're equivalent to physical caches, aren't they ?
r4k_flush_cache_mm() returns immediately if !cpu_has_dc_aliases.
---
Atsushi Nemoto
Quoted 8 lines · Atsushi Nemoto
Quoted 4 lines · Franck Bui-Huu
BTW,
flush_cache_mm()flushes (write back + invalidate ) the whole data cache unconditionnaly, but I'm wondering if it's really necessary for cpus which don't have any cache aliasing issues. After all they're equivalent to physical caches, aren't they ?
r4k_flush_cache_mm()returns immediately if !cpu_has_dc_aliases.
oops you're right, I was actually looking at local_r4k_flush_cache_mm().
thanks,
Franck