Re: [PATCH] Kill __bzero()

[Franck Bui-Huu <vagabon.xyz@gmail.com>]

Ralf Baechle wrote:
> On Tue, Nov 06, 2007 at 08:42:48AM +0100, Franck Bui-Huu wrote:
> 
>>> Older gcc used to generate significantly worse code for inline functions
>>> than for macros so Linux became a fairly excessive user of macros.  This
>>> has very much improved since, so these days inlines are prefered over
>>> macros where possible.
>> Yes but ISTR that gcc generates some calls to memset and since
>> builtin functions are disabled the final link failed if memset
>> is inlined. I'll try to reproduce...
> 

OK, using Cobalt config and the patch below, I get the following link
failure:

$ make

[snip]

  CC      init/version.o
  LD      init/built-in.o
  LD      .tmp_vmlinux1
kernel/built-in.o: In function `param_sysfs_init':
params.c:(.init.text+0x19ac): undefined reference to `memset'
params.c:(.init.text+0x19ac): relocation truncated to fit: R_MIPS_26 against `memset'
fs/built-in.o: In function `locks_remove_flock':
(.text+0x14518): undefined reference to `memset'
fs/built-in.o: In function `locks_remove_flock':
(.text+0x14518): relocation truncated to fit: R_MIPS_26 against `memset'
fs/built-in.o: In function `__blkdev_get':
block_dev.c:(.text+0x31d40): undefined reference to `memset'
block_dev.c:(.text+0x31d40): relocation truncated to fit: R_MIPS_26 against `memset'
block_dev.c:(.text+0x31d50): undefined reference to `memset'
block_dev.c:(.text+0x31d50): relocation truncated to fit: R_MIPS_26 against `memset'
make: *** [.tmp_vmlinux1] Error 1


> So both belt and suspenders then that is an inline/macro plus an outline
> version?
> 

I guess so.

>>>> Yes I noticed this. Actually I'm wondering if we couldn't add a new
>>>> function, fill_user() like the following:
>>>>
>>>> extern size_t fill_user(void __user *to, int c, size_t len);
>>> That's much better function name than the old __bzero - except that
>> Actually I named it '__fill_user', since it doesn't call access_ok().
>>
>>> __bzero effectivly took a long argument for the 2nd argument so 32-bit
>>> on 32-bit kernels and 64-bit on 64-bit kernels.
>> Isn't size_t meaning ?
>>
>> Perhaps in this case __kernel_size_t is better...
> 
> I wrote about the existing __bzero which takes the size_t length as third 
> argument and a long sized fill pattern as the second.
> 

Sorry I was confused (again) because of the invisible second parameter
of __bzero...

>> Yes I have a patchset which clean up a bit uaccess.h and does this but
>> it's under construction.  It actually tries to convert all macros into
>> inlines and the file is much more readable and as a bonus side we could
>> easily add __must_check annotations which are currently missing.
>>
>> I'll try to finish it this week but in the meantime can we just kill
>> __bzero or do you want me to include it in the future patchset ?
> 
> There is enough time until 2.6.25 to complete your cleanups; no more
> cleanups for 2.6.24.
> 

Sure.

		Franck
-- 8< --

diff --git a/arch/mips/lib/memset.S b/arch/mips/lib/memset.S
index 3f8b8b3..23f0490 100644
--- a/arch/mips/lib/memset.S
+++ b/arch/mips/lib/memset.S
@@ -54,7 +54,7 @@
  */
 	.set	noreorder
 	.align	5
-LEAF(memset)
+LEAF(__memset)
 	beqz		a1, 1f
 	 move		v0, a0			/* result */
 
diff --git a/include/asm-mips/string.h b/include/asm-mips/string.h
index 436e3ad..b2ef64c 100644
--- a/include/asm-mips/string.h
+++ b/include/asm-mips/string.h
@@ -132,7 +132,12 @@ strncmp(__const__ char *__cs, __const__ char *__ct, size_t __count)
 #endif /* CONFIG_32BIT */
 
 #define __HAVE_ARCH_MEMSET
-extern void *memset(void *__s, int __c, size_t __count);
+static inline void *memset(void *__s, int __c, size_t __count)
+{
+	extern void *__memset(void *s, int c, size_t len);
+
+	return __memset(__s, __c, __count);
+}
 
 #define __HAVE_ARCH_MEMCPY
 extern void *memcpy(void *__to, __const__ void *__from, size_t __n);