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);
|