On 02/15/2017 02:03 AM, Arnd Bergmann wrote:
> Hi Dmitry,
>
> I've come a little further on the stack size analysis after initially
> finding that gcc-7.0.1 is much better than the horrible stack frames
> we were seeing on gcc-7.0.0 (I found over 80kb in one case), I found
> more that remain with the newer compiler, but also analyzed the worst
> remaining ones down to two common helpers: typecheck() caused the
> worst remaining problem, but only in a few files like
> "drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5068:1: error: the frame
> size of 23768 bytes is larger than 1024 bytes". I think my fix should
> be able to make it in, but I'd give it some more testing. It also seems
> here that gcc asan-stack isn't too smart, as it adds checks to local
> variables we never use except for comparing their addresses. This may
> also impact min() and max(), which have the same check.
>
> READ_ONCE()/WRITE_ONCE() are used for atomic_t and caused the next worst
> offender (12688 byte stacks in lib/atomic64_test.c) as well as a lot
> of other instances that were over 2048 byte stacks. This one is a lot
> trickier as my the version from my patch is most likely not safe
> for all callers, and the helper has been extended to cover a lot of
> corner cases that my version destroys (it doesn't force aggregates
> to be accessed as scalars for example,
I think the following patch on top of yours should fix that for READ_ONCE().
WRITE_ONCE() probably can be fixed in a similar way, but I didn't try it.
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 10bca12..5d9dd63 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -301,10 +301,10 @@ static __always_inline void __write_once_size(volatile
void *p, void *res, int s
*/
#define __READ_ONCE(x, check) \
- __builtin_choose_expr(sizeof(x) == 1, *(volatile typeof(&(x)))&(x), \
- __builtin_choose_expr(sizeof(x) == 2, *(volatile typeof(&(x)))&(x), \
- __builtin_choose_expr(sizeof(x) == 4, *(volatile typeof(&(x)))&(x), \
- __builtin_choose_expr(sizeof(x) == sizeof(long), *(volatile
typeof(&(x)))&(x), \
+ __builtin_choose_expr(sizeof(x) == 1, (typeof(x))(__u64)*(volatile __u8
*)&(x), \
+ __builtin_choose_expr(sizeof(x) == 2, (typeof(x))(__u64)*(volatile
__u16 *)&(x), \
+ __builtin_choose_expr(sizeof(x) == 4, (typeof(x))(__u64)*(volatile
__u32 *)&(x), \
+ __builtin_choose_expr(sizeof(x) == 8, (typeof(x))(__u64)*(volatile
__u64 *)&(x), \
({union { typeof(x) __val; char __c[1]; } __u; \
if (check) \
__read_once_size(&(x), __u.__c, sizeof(x)); \
> probably also causes sparse
> warnings, and maybe doesn't even guarantee the "ONCE" semantics).
>
> I see that Andrey also provided a READ_ONCE_NOCHECK() helper that
> would also take care of this if used consistently, but it seems
> wrong to use that for all atomics. Any other ideas?
>
> Arnd
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 416b17e03016..b8018eddd757 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -310,14 +310,17 @@ static __always_inline void __write_once_size(volatile
> void *p, void *res, int s
> */
>
> #define __READ_ONCE(x, check)
> \
> -({ \
> - union { typeof(x) __val; char __c[1]; } __u; \
> + __builtin_choose_expr(sizeof(x) == 1, *(volatile typeof(&(x)))&(x), \
> + __builtin_choose_expr(sizeof(x) == 2, *(volatile typeof(&(x)))&(x), \
> + __builtin_choose_expr(sizeof(x) == 4, *(volatile typeof(&(x)))&(x), \
> + __builtin_choose_expr(sizeof(x) == sizeof(long), *(volatile
> typeof(&(x)))&(x), \
> + ({union { typeof(x) __val; char __c[1]; } __u; \
This should be under "if (check)" otherwise it breaks READ_ONCE_NOCHECK()
> if (check) \
> __read_once_size(&(x), __u.__c, sizeof(x)); \
> else \
> __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
> __u.__val; \
> -})
> + })))))
> #define READ_ONCE(x) __READ_ONCE(x, 1)
>
