Help with string.S
I found the source of the 4xx and 8xx troubles in the 2.4.xx kernel. The functions in arch/ppc/lib/string.S are broken for anything other than 32-byte cache lines. I am making the changes, but it would be nice to have someone else look at this as well. There are lots of assumptions outside of the apparent parameters that the cache is 32-bytes.
Thanks.
-- Dan
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Quoted 2 lines · Dan Malek
I found the source of the 4xx and 8xx troubles in the 2.4.xx kernel.
It wasn't really the source of the problem, but the functions are still not correct here. I converted the easy ones, the difficult one is copy_tofrom_user with all of its potential exception cases. I converted for 16 and 32 bytes lines, but the 64 and 128 bytes lines need some work.
Thanks.
-- Dan
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Hi Dan,
IIRC, I have sent you a patch that did the right thing for 16-byte cache lines at time of 2.3.99-pre5. So this is a known problem and had already been fixed. Of course, if there are caches with 64 or 128 byte lines, some more work needs to be done.
What gives me trouble is the fact that dcbz instruction in function arch/ppc/lib/string.S:__copy_tofrom_user does not seem to work for me. But the function works fine if I remove that instruction. Has anybody else experienced similar problems ?
Any suggestions welcome,
Daniel Marmier
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Daniel Marmier writes:
Daniel> IIRC, I have sent you a patch that did the right thing for 16-byte Daniel> cache lines at time of 2.3.99-pre5. So this is a known problem and Daniel> had already been fixed. Of course, if there are caches with 64 or Daniel> 128 byte lines, some more work needs to be done.
Current 64-bit PowerPC chips use a cacheline size of 128 bytes.
Assuming 32 bytes or 32 bytes and 16 bytes or any small number of values is a mistake.
David
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Quoted 2 lines · Daniel Marmier
IIRC, I have sent you a patch that did the right thing for 16-byte cache lines at time of 2.3.99-pre5.
Now I remember :-). I see too much code from too many places every day!
Quoted 2 lines
...... Of course, if there are caches with 64 or
128 byte lines, some more work needs to be done.
Lots more :-).
Quoted 2 lines
What gives me trouble is the fact that dcbz instruction in function arch/ppc/lib/string.S:__copy_tofrom_user does not seem to work for me.
These are becoming a pain in the ass instructions. Has anyone ever done some performance analysis to see what we really gain here in real life? Sure, locally and logically you can make an intuitive argument, but we are sure fetching lots of instructions just to get this aligned, and further to actually move the data.
These instructions certainly don't work on uncached memory space, causing the alignment exception and probably horrible performance without people knowing. These instructions used to cause the exception on the early MPC8xx processors when copyback cache wasn't enabled. Today, the newer silicon doesn't fault at all regardless of cache mode. I guess I need to determine what is really happening. Nothing would be fine, but it appears _something_ (usually incorrect) happens.
Quoted 1 line
But the function works fine if I remove that instruction.
I'm still a C code fan:
for(i=0; i<count; i++) *d++ = *s++;
...and let the compiler guys make it go fast :-).
You know, we could make this even faster by using the Altivec and the new cache streaming modes on the 7400 processors :-). I've tested this in applications. It really works.
-- Dan
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Quoted 6 lines · Dan Malek
These are becoming a pain in the ass instructions. Has anyone ever done some performance analysis to see what we really gain here in real life? Sure, locally and logically you can make an intuitive argument, but we are sure fetching lots of instructions just to get this aligned, and further to actually move the data.
Quoted 7 lines
These instructions certainly don't work on uncached memory space, causing the alignment exception and probably horrible performance without people knowing. These instructions used to cause the exception on the early MPC8xx processors when copyback cache wasn't enabled. Today, the newer silicon doesn't fault at all regardless of cache mode. I guess I need to determine what is really happening. Nothing would be fine, but it appears _something_ (usually incorrect) happens.
I have seen this happen on cacheable memory with copyback enabled. The dcbz-memcpy caused the destination to be zeroed, IIRC.
Quoted 7 lines
Quoted 1 line
But the function works fine if I remove that instruction.
I'm still a C code fan:
for(i=0; i<count; i++) *d++ = *s++;...and let the compiler guys make it go fast :-).
That would be cool, but I am sure the asm funcs perform much better. I'll try to do some benchmarking if I have time.
Daniel M.
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Quoted 7 lines · Dan Malek
Quoted 2 lines
What gives me trouble is the fact that dcbz instruction in function arch/ppc/lib/string.S:__copy_tofrom_user does not seem to work for me.
These are becoming a pain in the ass instructions. Has anyone ever done some performance analysis to see what we really gain here in real life? Sure, locally and logically you can make an intuitive argument, but we are sure fetching lots of instructions just to get this aligned, and further to actually move the data.
The 7xx(x) processors don't have the alignment handler set up to cover this problem in 2.2, so they just get an oops when somebody writes to uncached memory, like a framebuffer device. This could probably be solved by starting the function with a test of the address, and using a version without cache operations for target addresses above the kernel image of memory.
Or by removing the cache operations. Even if they stay, could they be a compilation time optimisation for particular processors?
Quoted 3 lines
You know, we could make this even faster by using the Altivec and the new cache streaming modes on the 7400 processors :-). I've tested this in applications. It really works.
The 7400 certainly doesn't need the dcbz, as it will perform an implicit allocation if the entire cache line is written by store instructions.
- Adrian Cox, AG Electronics
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Quoted 1 line · Adrian Cox
The 7xx(x) processors don't have the alignment handler set up ....
Paul and I (and possibly others) conspired and added this in the late 2.3.xx kernels for all processors. It had been floating around for the MPC8xx processors, I hit it again on the 8260, and we just made the code generic for all processors. It "fixes" alignment faults and will also zero memory on a dcbz fault. Hmmm, I wonder if this code actually gets called and if it still does the right thing? I'll check it again.
Quoted 2 lines
Or by removing the cache operations. Even if they stay, could they be a compilation time optimisation for particular processors?
While the code wasn't really correct for anything but 32 byte cache lines, it should work correctly on 16 byte lines. You don't get the performance increase as the dcbz is only performed every other cache line. However, like David mentioned, it really is broken for 64 and 128 byte cache lines. Here, you zero a long line, but only fill 32 bytes of data. You end up with a nearly zero filled buffer.
Anyway, enough talk, it has to be fixed. I'll do the best I can. I would like to remove the assumption in copy_tofrom_user that we can fault in so many places in the cache line. Considering all of the alignment restrictions, it seems to me you will only fault on the first access to the cache line (it isn't like you are going to cross a page boundary in the middle of a line). This would simplify the function and make for a much smaller exception table.
Quoted 2 lines
The 7400 certainly doesn't need the dcbz, as it will perform an implicit allocation if the entire cache line is written by store instructions.
No, but those cache streaming instructions and data move cache hints really do something. It was my attempt at humor, you see :-).
-- Dan
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/
Quoted 6 lines · Daniel Marmier
Quoted 1 line · Dan Malek
These are becoming a pain in the ass instructions.
I have seen this happen on cacheable memory with copyback enabled. The dcbz-memcpy caused the destination to be zeroed, IIRC.
OK, I think I am bailing out here. For some reason, if I remove the 'dcbz' instructions on the MPC8xx processor the world is just a better place. I don't know why, maybe because of some of the TLB mapping, but I can't find a reason.
I am going to put #ifdef CONFIG_8xx around the dcbz instructions, and where they are actually used to zero-fill memory I will use store operations. The other option is to make the changes at a higher level (like make 'clear_page' call 'memset' with 0), but I think the direct assembly changes are preferable. Suggestions welcome.
I'll keep looking for a better solution, but I can't hold up others trying to use this kernel.
Thanks.
-- Dan
** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/