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, 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/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h index fcc5cd387fd1..0c243dd569fe 100644 --- a/arch/x86/include/asm/switch_to.h +++ b/arch/x86/include/asm/switch_to.h @@ -30,7 +30,7 @@ static inline void prepare_switch_to(struct task_struct *prev, * * To minimize cache pollution, just follow the stack pointer. */ - READ_ONCE(*(unsigned char *)next->thread.sp); + (void)READ_ONCE(*(unsigned char *)next->thread.sp); #endif } diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 01157d6e8cfe..dc677c7c22be 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -222,8 +222,8 @@ void ovl_dentry_update(struct dentry *dentry, struct dentry *upperdentry) void ovl_inode_init(struct inode *inode, struct inode *realinode, bool is_upper) { - WRITE_ONCE(inode->i_private, (unsigned long) realinode | - (is_upper ? OVL_ISUPPER_MASK : 0)); + WRITE_ONCE(inode->i_private, (void *)((unsigned long) realinode | + (is_upper ? OVL_ISUPPER_MASK : 0))); } void ovl_inode_update(struct inode *inode, struct inode *upperinode) @@ -231,7 +231,7 @@ void ovl_inode_update(struct inode *inode, struct inode *upperinode) WARN_ON(!upperinode); WARN_ON(!inode_unhashed(inode)); WRITE_ONCE(inode->i_private, - (unsigned long) upperinode | OVL_ISUPPER_MASK); + (void *)((unsigned long) upperinode | OVL_ISUPPER_MASK)); if (!S_ISDIR(upperinode->i_mode)) __insert_inode_hash(inode, (unsigned long) upperinode); } 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; \ 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) /* @@ -326,13 +329,19 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s */ #define READ_ONCE_NOCHECK(x) __READ_ONCE(x, 0) -#define WRITE_ONCE(x, val) \ -({ \ - union { typeof(x) __val; char __c[1]; } __u = \ - { .__val = (__force typeof(x)) (val) }; \ - __write_once_size(&(x), __u.__c, sizeof(x)); \ - __u.__val; \ -}) +#define WRITE_ONCE(x, val) \ +( \ + __builtin_choose_expr(sizeof(x) == 1, *(volatile typeof(&(x)))&(x) = (val), \ + __builtin_choose_expr(sizeof(x) == 2, *(volatile typeof(&(x)))&(x) = (val), \ + __builtin_choose_expr(sizeof(x) == 4, *(volatile typeof(&(x)))&(x) = (val), \ + __builtin_choose_expr(sizeof(x) == sizeof(long), *(volatile typeof(&(x)))&(x) = (val), \ + ({ \ + typeof(x) __val = (val); \ + barrier(); \ + __builtin_memcpy((void *)&x, (void *)&__val, sizeof(__val)); \ + barrier(); \ + }))))) \ +) #endif /* __KERNEL__ */ diff --git a/include/linux/typecheck.h b/include/linux/typecheck.h index eb5b74a575be..adb1579fa5f0 100644 --- a/include/linux/typecheck.h +++ b/include/linux/typecheck.h @@ -5,12 +5,7 @@ * Check at compile time that something is of a particular type. * Always evaluates to 1 so you may use it easily in comparisons. */ -#define typecheck(type,x) \ -({ type __dummy; \ - typeof(x) __dummy2; \ - (void)(&__dummy == &__dummy2); \ - 1; \ -}) +#define typecheck(type,x) ({(void)((typeof(type) *)NULL == (typeof(x) *)NULL); 1;}) /* * Check at compile time that 'function' is a certain type, or is a pointer