arm: lib: memcpy: Do not copy to same address
PATCH 1/1 arm: lib: memcpy: Do not copy to same address
In some cases (e.g. bootm with a elf payload which is already at the right position) there is a in place copy of data to the same address. Catching this saves some ms while booting.
--- arch/arm/lib/memcpy.S | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
arch/arm/lib/memcpy.S +3 -0
@@ -60,6 +60,9 @@ .globl memcpy memcpy: + cmp r0, r1 + moveq pc, lr + enter r4, lr subs r2, r2, #4
Signature · 1 line
1.7.0.4
PATCH 1/1 arm: lib: memcpy: Do not copy to same address
Am 23.05.2011 11:06, schrieb Matthias Weisser:
Quoted 14 lines
In some cases (e.g. bootm with a elf payload which is already at the right position) there is a in place copy of data to the same address. Catching this saves some ms while booting.
Signed-off-by Matthias Weisser<[email protected]>--- arch/arm/lib/memcpy.S | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)arch/arm/lib/memcpy.S +0
-0
arch/arm/lib/memcpy.S line 68
Quoted 9 lines
arch/arm/lib/memcpy.S +3
-0@@ -60,6 +60,9 @@ .globl memcpy memcpy: + cmp r0, r1 + moveq pc, lr + enter r4, lr subs r2, r2, #4
The standard clearly say to both memory regions should not overlap when memcpy() is used, so I would say this is the wrong place to fix that.
Regards,
Alexander
PATCH 1/1 arm: lib: memcpy: Do not copy to same address
Am 23.05.2011 11:30, schrieb Alexander Holler:
Quoted 23 lines
Am 23.05.2011 11:06, schrieb Matthias Weisser:
Quoted 22 lines
In some cases (e.g. bootm with a elf payload which is already at the right position) there is a in place copy of data to the same address. Catching this saves some ms while booting.
Signed-off-by Matthias Weisser<[email protected]>--- arch/arm/lib/memcpy.S | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)arch/arm/lib/memcpy.S +3
-0@@ -60,6 +60,9 @@ .globl memcpy memcpy: + cmp r0, r1 + moveq pc, lr + enter r4, lr
Quoted 4 lines
arch/arm/lib/memcpy.S line 68
Quoted 1 line
arch/arm/lib/memcpy.S +0
-0subs r2, r2, #4The standard clearly say to both memory regions should not overlap when
memcpy()is used, so I would say this is the wrong place to fix that.
Well, real world applications do this. And these two instructions shouldn't hurt a lot.
I first send a patch fixing only "my" problem in cmd_elf.c but Wolfgang suggested to do this globally. Please see
http://www.mail-archive.com/u-boot at lists.denx.de/msg50612.html as reference.
Matthias
PATCH 1/1 arm: lib: memcpy: Do not copy to same address
Am 23.05.2011 11:46, schrieb Matthias Wei?er:
Quoted 5 lines
Quoted 2 lines
The standard clearly say to both memory regions should not overlap when
memcpy()is used, so I would say this is the wrong place to fix that.Well, real world applications do this. And these two instructions shouldn't hurt a lot.
Real bugs to this. Just see e.g the long discussion about some changes fo memcpy done in the glibc lately and what that did for flash-users because flash seemed to the same stupid stuff.
Regards,
Alexander
PATCH 1/1 arm: lib: memcpy: Do not copy to same address
In reply to Alexander HollerLe 23/05/2011 11:30, Alexander Holler a ?crit :
Quoted 23 lines
Am 23.05.2011 11:06, schrieb Matthias Weisser:
Quoted 22 lines
In some cases (e.g. bootm with a elf payload which is already at the right position) there is a in place copy of data to the same address. Catching this saves some ms while booting.
Signed-off-by Matthias Weisser<[email protected]>--- arch/arm/lib/memcpy.S | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)arch/arm/lib/memcpy.S +3
-0@@ -60,6 +60,9 @@ .globl memcpy memcpy: + cmp r0, r1 + moveq pc, lr + enter r4, lr
Quoted 4 lines
arch/arm/lib/memcpy.S line 68
Quoted 1 line
arch/arm/lib/memcpy.S +0
-0subs r2, r2, #4The standard clearly say to both memory regions should not overlap when
memcpy()is used, so I would say this is the wrong place to fix that.
I think the intent here is not to enforce the standard but to handle an actual, and degenerate, copy request in the most efficient manner, and in that respect, the patch does its job.
Besides, if the patch was about enforcing the standard, then I would consider it highly more efficient to check the areas once in the memcpy function than duplicating this check before each call to the function, considering that the place where the copy happens is not necessarily the one where the source and destination were computed.
Quoted 3 lines
Regards,
Alexander
Amicalement,
Signature · 1 line
Albert.
PATCH 1/1 arm: lib: memcpy: Do not copy to same address
Hello,
Am 23.05.2011 11:49, schrieb Albert ARIBAUD:
Quoted 7 lines
Quoted 2 lines
The standard clearly say to both memory regions should not overlap when
memcpy()is used, so I would say this is the wrong place to fix that.I think the intent here is not to enforce the standard but to handle an actual, and degenerate, copy request in the most efficient manner, and in that respect, the patch does its job.
Quoted 5 lines
Besides, if the patch was about enforcing the standard, then I would consider it highly more efficient to check the areas once in the memcpy function than duplicating this check before each call to the function, considering that the place where the copy happens is not necessarily the one where the source and destination were computed.
A fool proof solution would be to always use memmove() and get rid of memcpy(). But checking for overlapped regions in memcpy() is imho the wrong way. This just leads to more possible wrong code which uses memcpy() when it should use memmove().
Regards,
Alexander
PATCH 1/1 arm: lib: memcpy: Do not copy to same address
In reply to Albert ARIBAUDDear Albert
Am 23.05.2011 11:49, schrieb Albert ARIBAUD:
Quoted 24 lines
Le 23/05/2011 11:30, Alexander Holler a ?crit :
Quoted 23 lines
Am 23.05.2011 11:06, schrieb Matthias Weisser:
Quoted 22 lines
In some cases (e.g. bootm with a elf payload which is already at the right position) there is a in place copy of data to the same address. Catching this saves some ms while booting.
Signed-off-by Matthias Weisser<[email protected]>--- arch/arm/lib/memcpy.S | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)arch/arm/lib/memcpy.S +3
-0@@ -60,6 +60,9 @@ .globl memcpy memcpy: + cmp r0, r1 + moveq pc, lr + enter r4, lr
Quoted 8 lines
Quoted 4 lines
arch/arm/lib/memcpy.S line 68
Quoted 1 line
arch/arm/lib/memcpy.S +0
-0subs r2, r2, #4The standard clearly say to both memory regions should not overlap when
memcpy()is used, so I would say this is the wrong place to fix that.I think the intent here is not to enforce the standard but to handle an actual, and degenerate, copy request in the most efficient manner, and in that respect, the patch does its job.
Can this patch go in or do I need to change anything? I really would like to see it in mainline.
Regards,
Matthias
PATCH 1/1 arm: lib: memcpy: Do not copy to same address
In reply to Matthias WeisserQuoted 14 lines · Matthias Weisser
In some cases (e.g. bootm with a elf payload which is already at the right position) there is a in place copy of data to the same address. Catching this saves some ms while booting.
Signed-off-by Matthias Weisser<[email protected]>--- arch/arm/lib/memcpy.S | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)arch/arm/lib/memcpy.S +0
-0
arch/arm/lib/memcpy.S line 68
Quoted 9 lines
arch/arm/lib/memcpy.S +3
-0@@ -60,6 +60,9 @@ .globl memcpy memcpy: + cmp r0, r1 + moveq pc, lr + enter r4, lr subs r2, r2, #4
Applied to u-boot-arm/master, thanks.
Amicalement,
Signature · 1 line
Albert.