csky: uaccess.h: Coding convention with asm generic

From: Guo Ren <[email protected]>

Using asm-generic/uaccess.h to prevent duplicated code:
- Add user_addr_max which mentioned in generic uaccess.h
- Remove custom definitions of KERNEL/USER_DS, get/set_fs, uaccess_kerenl

From: Guo Ren <[email protected]>

Using asm-generic/uaccess.h to prevent duplicated code:
- Add user_addr_max which mentioned in generic uaccess.h
- Remove custom definitions of KERNEL/USER_DS, get/set_fs, uaccess_kerenl
- Using generic extable.h instead of custom definitions in uaccess.h

Signed-off-by Guo Ren <[email protected]>

Cc Arnd Bergmann <[email protected]>

Quoted 11 lines · On Wed, Apr 21, 2021 at 08:54:15AM +0000, [email protected] wrote:

From: Guo Ren <[email protected]>

Using asm-generic/uaccess.h to prevent duplicated code:
- Add user_addr_max which mentioned in generic uaccess.h
- Remove custom definitions of KERNEL/USER_DS, get/set_fs, uaccess_kerenl
- Using generic extable.h instead of custom definitions in uaccess.h

Signed-off-by Guo Ren <[email protected]>

Cc Arnd Bergmann <[email protected]>

Building csky:tinyconfig ... failed
--------------
Error log:
csky-linux-ld: fs/readdir.o: in function `__put_user_fn':
readdir.c:(.text+0x7c): undefined reference to `__put_user_bad'
csky-linux-ld: fs/readdir.o: in function `$d':
readdir.c:(.text+0x1bc): undefined reference to `__put_user_bad' make[1]: *** [Makefile:1277: vmlinux] Error 1
make: *** [Makefile:222: __sub-make] Error 2

Bisect log attached.

Guenter

---
# bad: [3f1fee3e7237347f09a2c7fa538119e6d9ea4b84] Add linux-next specific files for 20210426 # good: [9f4ad9e425a1d3b6a34617b8ea226d56a119a717] Linux 5.12
git bisect start 'HEAD' 'v5.12'
# bad: [bb8f486776983897309645c98705670c3d2a16e5] Merge remote-tracking branch 'crypto/master' git bisect bad bb8f486776983897309645c98705670c3d2a16e5
# bad: [6ab4f4364c450991a476eef5bc57bef3586354ed] Merge remote-tracking branch 'jc_docs/docs-next' git bisect bad 6ab4f4364c450991a476eef5bc57bef3586354ed
# bad: [fd73cab0b2a046136842b23f027dae3686588ba5] Merge remote-tracking branch 'parisc-hd/for-next' git bisect bad fd73cab0b2a046136842b23f027dae3686588ba5
# good: [f0e6103e023e0ede67848ddcd6b07044574f4fd3] soc: document merges
git bisect good f0e6103e023e0ede67848ddcd6b07044574f4fd3
# good: [70361dc0add47d3818acf9c33718ce7395f8aaa5] Merge remote-tracking branch 'arm-soc/for-next' git bisect good 70361dc0add47d3818acf9c33718ce7395f8aaa5
# good: [f62ad9f6e1100e3a1b6ca7a004fd5a972ff768df] Merge remote-tracking branch 'ti-k3/ti-k3-next' git bisect good f62ad9f6e1100e3a1b6ca7a004fd5a972ff768df
# bad: [b3b33dda4fd25e201c77f0ce9277dd34f31e86ce] Merge remote-tracking branch 'h8300/h8300-next' git bisect bad b3b33dda4fd25e201c77f0ce9277dd34f31e86ce
# good: [6a861bd8cf3c96f5825d031732e365b7721a84a5] Merge branch 'clk-qcom' into clk-next git bisect good 6a861bd8cf3c96f5825d031732e365b7721a84a5
# good: [1dd129f1deec0606fb70992521a7e5bcd2f85c69] Merge branch 'clk-qcom' into clk-next git bisect good 1dd129f1deec0606fb70992521a7e5bcd2f85c69
# good: [8808515be0ed4e33de9bfdc65f4c1b547ee11065] h8300: Replace <linux/clk-provider.h> by <linux/of_clk.h> git bisect good 8808515be0ed4e33de9bfdc65f4c1b547ee11065
# good: [e27d3ecdeb8923f35cb856fd20be14256aaa7575] Merge remote-tracking branch 'clk/clk-next' git bisect good e27d3ecdeb8923f35cb856fd20be14256aaa7575
# bad: [d3900e8d918f8fbd1366b9c2998e2830e66a0081] csky: uaccess.h: Coding convention with asm generic git bisect bad d3900e8d918f8fbd1366b9c2998e2830e66a0081
# good: [0b1f557a1fa02174a982f557581e348d91987ec6] csky: Fixup typos
git bisect good 0b1f557a1fa02174a982f557581e348d91987ec6
# good: [8bfe70e696584deeed1de1bcbfcde405aa1a1344] csky: fix syscache.c fallthrough warning git bisect good 8bfe70e696584deeed1de1bcbfcde405aa1a1344

# first bad commit: [d3900e8d918f8fbd1366b9c2998e2830e66a0081] csky: uaccess.h: Coding convention with asm generic

Quoted 14 lines · Guenter Roeck

Quoted 11 lines · On Wed, Apr 21, 2021 at 08:54:15AM +0000, [email protected] wrote:

From: Guo Ren <[email protected]>

Using asm-generic/uaccess.h to prevent duplicated code:
- Add user_addr_max which mentioned in generic uaccess.h
- Remove custom definitions of KERNEL/USER_DS, get/set_fs, uaccess_kerenl
- Using generic extable.h instead of custom definitions in uaccess.h

Signed-off-by Guo Ren <[email protected]>

Cc Arnd Bergmann <[email protected]>

Quoted 9 lines

Building csky:tinyconfig ... failed
--------------
Error log:
csky-linux-ld: fs/readdir.o: in function `__put_user_fn':
readdir.c:(.text+0x7c): undefined reference to `__put_user_bad'
csky-linux-ld: fs/readdir.o: in function `$d':
readdir.c:(.text+0x1bc): undefined reference to `__put_user_bad' make[1]: *** [Makefile:1277: vmlinux] Error 1
make: *** [Makefile:222: __sub-make] Error 2

It's a bug, I can't put __put_user_bad in __put_user_fn, and __put_user has done that:

/*
* These are the main single-value transfer routines. They automatically
* use the right size if we just have the right pointer type.
* This version just falls back to copy_{from,to}_user, which should
* provide a fast-path for small values.
*/
#define __put_user(x, ptr) \

({                                                              \
        __typeof__(*(ptr)) __x = (x);                           \
        int __pu_err = -EFAULT;                                 \
        __chk_user_ptr(ptr);                                    \
        switch (sizeof (*(ptr))) {                              \
        case 1:                                                 \
        case 2:                                                 \
        case 4:                                                 \
        case 8:                                                 \
                __pu_err = __put_user_fn(sizeof (*(ptr)),       \
                                         ptr, &__x);            \
                break;                                          \
        default:                                                \
                __put_user_bad();                               \
                break;                                          \
         }                                                      \
        __pu_err;                                               \

})

Actually, please don't use the asm-generic __put_user version based on copy_to_user, we probably have killed it off long ago.

We might want to come up with a new version of asm-generic/uaccess.h that actually makes it easier to have a sane per-architecture implementation of the low-level accessors without set_fs().

I've added Christoph to Cc here, he probably has some ideas on where we should be heading.

One noteworthy aspect is that almost nothing users the low-level __get_user()/__put_user() helpers any more outside of architecture specific code, so we may not need to have separate versions for much longer.

Quoted 2 lines · Arnd Bergmann

Actually, please don't use the asm-generic __put_user version based on copy_to_user, we probably have killed it off long ago.

Yes, they are horrible.

Quoted 6 lines

We might want to come up with a new version of asm-generic/uaccess.h that actually makes it easier to have a sane per-architecture implementation of the low-level accessors without set_fs().

I've added Christoph to Cc here, he probably has some ideas on where we should be heading.

I think asm-generic/uaccess.h pretty much only makes sense for nommu. For that case we can just kill the __{get,put}_user_fn indirection. I actually have work for that in an old branch.

Trying to use any of asm-generic/uaccess.h for MMU based kernel is just asking for trouble.

Quoted 5 lines

One noteworthy aspect is that almost nothing users the low-level __get_user()/__put_user() helpers any more outside of architecture specific code, so we may not need to have separate versions for much longer.

Al has been trying to kill them off entirely for a while, and I hope he'll eventually succeed. That being said the difference should be that the __ versions just skip the access_ok, so having both is fairly trivial to implement.

Quoted 6 lines · Arnd Bergmann

We might want to come up with a new version of asm-generic/uaccess.h that actually makes it easier to have a sane per-architecture implementation of the low-level accessors without set_fs().

I've added Christoph to Cc here, he probably has some ideas on where we should be heading.

I think asm-generic/uaccess.h pretty much only makes sense for nommu. For that case we can just kill the __{get,put}_user_fn indirection. I actually have work for that in an old branch.

Trying to use any of asm-generic/uaccess.h for MMU based kernel is just asking for trouble.

Quoted 4 lines

One noteworthy aspect is that almost nothing users the low-level __get_user()/__put_user() helpers any more outside of architecture specific code, so we may not need to have separate versions for much longer.

Al has been trying to kill them off entirely for a while, and I hope he'll eventually succeed. That being said the difference should be that the __ versions just skip the access_ok, so having both is fairly trivial to implement.

Quoted 51 lines · Guo Ren

Quoted 14 lines · Guenter Roeck

Quoted 11 lines · On Wed, Apr 21, 2021 at 08:54:15AM +0000, [email protected] wrote:

From: Guo Ren <[email protected]>

Using asm-generic/uaccess.h to prevent duplicated code:
- Add user_addr_max which mentioned in generic uaccess.h
- Remove custom definitions of KERNEL/USER_DS, get/set_fs, uaccess_kerenl
- Using generic extable.h instead of custom definitions in uaccess.h

Signed-off-by Guo Ren <[email protected]>

Cc Arnd Bergmann <[email protected]>

Quoted 9 lines

Building csky:tinyconfig ... failed
--------------
Error log:
csky-linux-ld: fs/readdir.o: in function `__put_user_fn':
readdir.c:(.text+0x7c): undefined reference to `__put_user_bad'
csky-linux-ld: fs/readdir.o: in function `$d':
readdir.c:(.text+0x1bc): undefined reference to `__put_user_bad' make[1]: *** [Makefile:1277: vmlinux] Error 1
make: *** [Makefile:222: __sub-make] Error 2

It's a bug, I can't put __put_user_bad in __put_user_fn, and __put_user has done that:

/*
* These are the main single-value transfer routines. They automatically
* use the right size if we just have the right pointer type.
* This version just falls back to copy_{from,to}_user, which should
* provide a fast-path for small values.
*/
#define __put_user(x, ptr) \

({                                                              \
        __typeof__(*(ptr)) __x = (x);                           \
        int __pu_err = -EFAULT;                                 \
        __chk_user_ptr(ptr);                                    \
        switch (sizeof (*(ptr))) {                              \
        case 1:                                                 \
        case 2:                                                 \
        case 4:                                                 \
        case 8:                                                 \
                __pu_err = __put_user_fn(sizeof (*(ptr)),       \
                                         ptr, &__x);            \
                break;                                          \
        default:                                                \
                __put_user_bad();                               \
                break;                                          \
         }                                                      \
        __pu_err;                                               \

Quoted 1 line

})

Actually, please don't use the asm-generic __put_user version based on copy_to_user, we probably have killed it off long ago.

We might want to come up with a new version of asm-generic/uaccess.h that actually makes it easier to have a sane per-architecture implementation of the low-level accessors without set_fs().

I've added Christoph to Cc here, he probably has some ideas on where we should be heading.

One noteworthy aspect is that almost nothing users the low-level __get_user()/__put_user() helpers any more outside of architecture specific code, so we may not need to have separate versions for much longer.