Hi Paul, On Wed, Aug 14, 2019 at 10:03:39AM -0700, Paul Walmsley wrote: > Hi Nick, > > On Wed, 14 Aug 2019, Nick Hu wrote: > > > On Wed, Aug 14, 2019 at 10:22:15AM +0800, Paul Walmsley wrote: > > > On Tue, 13 Aug 2019, Palmer Dabbelt wrote: > > > > > > > On Mon, 12 Aug 2019 08:04:46 PDT (-0700), Christoph Hellwig wrote: > > > > > On Wed, Aug 07, 2019 at 03:19:14PM +0800, Nick Hu wrote: > > > > > > There are some features which need this string operation for > > > > > > compilation, > > > > > > like KASAN. So the purpose of this porting is for the features like > > > > > > KASAN > > > > > > which cannot be compiled without it. > > > > > > > > > > > > KASAN's string operations would replace the original string > > > > > > operations and > > > > > > call for the architecture defined string operations. Since we don't > > > > > > have > > > > > > this in current kernel, this patch provides the implementation. > > > > > > > > > > > > This porting refers to the 'arch/nds32/lib/memmove.S'. > > > > > > > > > > This looks sensible to me, although my stringop asm is rather rusty, > > > > > so just an ack and not a real review-by: > > > > > > > > FWIW, we just write this in C everywhere else and rely on the compiler > > > > to > > > > unroll the loops. I always prefer C to assembly when possible, so I'd > > > > prefer > > > > if we just adopt the string code from newlib. We have a RISC-V-specific > > > > memcpy in there, but just use the generic memmove. > > > > > > > > Maybe the best bet here would be to adopt the newlib memcpy/memmove as > > > > generic > > > > Linux functions? They're both in C so they should be fine, and they > > > > both look > > > > faster than what's in lib/string.c. Then everyone would benefit and we > > > > don't > > > > need this tricky RISC-V assembly. Also, from the look of it the newlib > > > > code > > > > is faster because the inner loop is unrolled. > > > > > > There's a generic memmove implementation in the kernel already: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/string.h#n362 > > > > > > Nick, could you tell us more about why the generic memmove() isn't > > > suitable? > > > > KASAN has its own string operations(memcpy/memmove/memset) because it needs > > to > > hook some code to check memory region. It would undefined the original > > string > > operations and called the string operations with the prefix '__'. But the > > generic string operations didn't declare with the prefix. Other archs with > > KASAN support like arm64 and xtensa all have their own string operations and > > defined with the prefix. > > Thanks for the explanation. What do you think about Palmer's idea to > define a generic C set of KASAN string operations, derived from the newlib > code? > > > - Paul
That sounds good to me. But it should be another topic. We need to investigate it further about replacing something generic and fundamental in lib/string.c with newlib C functions. Some blind spots may exist. So I suggest, let's consider KASAN for now. Nick
