| Message ID | 20240709105242.63299-1-flyingpeng@tencent.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | infiniband/hw/ocrdma: increase frame warning limit in verifier when using KASAN or KCSAN | expand |
On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote: > From: Peng Hao <flyingpeng@tencent.com> > > When building kernel with clang, which will typically > have sanitizers enabled, there is a warning about a large stack frame. > > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than] > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer, > ^ Please fix it, not hide it. Thanks > > Increase the frame size by 20% to set. > > Signed-off-by: Peng Hao <flyingpeng@tencent.com> > --- > drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile > index 14fba95021d8..a1e9fcc04751 100644 > --- a/drivers/infiniband/hw/ocrdma/Makefile > +++ b/drivers/infiniband/hw/ocrdma/Makefile > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet > > obj-$(CONFIG_INFINIBAND_OCRDMA) += ocrdma.o > > +ifneq ($(CONFIG_FRAME_WARN),0) > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664 > +endif > +endif > + > ocrdma-y := ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o > -- > 2.27.0 > >
On Tue, Jul 09, 2024 at 03:27:37PM +0300, Leon Romanovsky wrote: > On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote: > > From: Peng Hao <flyingpeng@tencent.com> > > > > When building kernel with clang, which will typically > > have sanitizers enabled, there is a warning about a large stack frame. > > > > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than] > > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer, > > ^ > > Please fix it, not hide it. Agreed, this is far from an acceptable solution. No details were provided around compiler, architecture, or configuration, so I can only speculate what is happening here. From reading the code, I suspect that ocrdma_add_stat() is getting inlined into all of its callsites but the stack slot for buff[128] is not getting reused, which may be related to a missing lifetime marker like [1] or sanitizer instrumentation. I am guessing that marking ocrdma_dbgfs_ops_read() as noinline_for_stack would resolve this. static noinline_for_stack int ocrdma_add_stat(char *start, char *pcur, If this is not tolerable for all configurations, it could be made more pointed with something like static #if defined(CONFIG_CC_IS_CLANG) && (defined(CONFIG_KASAN) || defined(CONFIG_KCSAN)) noinline_for_stack #endif int ocrdma_add_stat(char *start, char *pcur, but I am aware that is quite ugly. [1]: https://github.com/llvm/llvm-project/issues/38157#issuecomment-1756321571 Cheers, Nathan > > Increase the frame size by 20% to set. > > > > Signed-off-by: Peng Hao <flyingpeng@tencent.com> > > --- > > drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile > > index 14fba95021d8..a1e9fcc04751 100644 > > --- a/drivers/infiniband/hw/ocrdma/Makefile > > +++ b/drivers/infiniband/hw/ocrdma/Makefile > > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet > > > > obj-$(CONFIG_INFINIBAND_OCRDMA) += ocrdma.o > > > > +ifneq ($(CONFIG_FRAME_WARN),0) > > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) > > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664 > > +endif > > +endif > > + > > ocrdma-y := ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o > > -- > > 2.27.0 > > > >
On Tue, Jul 09, 2024 at 02:26:08PM -0700, Nathan Chancellor wrote: > On Tue, Jul 09, 2024 at 03:27:37PM +0300, Leon Romanovsky wrote: > > On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote: > > > From: Peng Hao <flyingpeng@tencent.com> > > > > > > When building kernel with clang, which will typically > > > have sanitizers enabled, there is a warning about a large stack frame. > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than] > > > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer, > > > ^ > > > > Please fix it, not hide it. > > Agreed, this is far from an acceptable solution. No details were > provided around compiler, architecture, or configuration, so I can only > speculate what is happening here. From reading the code, I suspect that > ocrdma_add_stat() is getting inlined into all of its callsites but the > stack slot for buff[128] is not getting reused, which may be related to > a missing lifetime marker like [1] or sanitizer instrumentation. I am > guessing that marking ocrdma_dbgfs_ops_read() as noinline_for_stack > would resolve this. > > static noinline_for_stack int ocrdma_add_stat(char *start, char *pcur, This is a good solution, but first let's see if the author can provide more details. > > If this is not tolerable for all configurations, it could be made more > pointed with something like > > static > #if defined(CONFIG_CC_IS_CLANG) && (defined(CONFIG_KASAN) || defined(CONFIG_KCSAN)) > noinline_for_stack > #endif > int ocrdma_add_stat(char *start, char *pcur, > > but I am aware that is quite ugly. > > [1]: https://github.com/llvm/llvm-project/issues/38157#issuecomment-1756321571 > > Cheers, > Nathan > > > > Increase the frame size by 20% to set. > > > > > > Signed-off-by: Peng Hao <flyingpeng@tencent.com> > > > --- > > > drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile > > > index 14fba95021d8..a1e9fcc04751 100644 > > > --- a/drivers/infiniband/hw/ocrdma/Makefile > > > +++ b/drivers/infiniband/hw/ocrdma/Makefile > > > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet > > > > > > obj-$(CONFIG_INFINIBAND_OCRDMA) += ocrdma.o > > > > > > +ifneq ($(CONFIG_FRAME_WARN),0) > > > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) > > > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664 > > > +endif > > > +endif > > > + > > > ocrdma-y := ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o > > > -- > > > 2.27.0 > > > > > >
On Wed, Jul 10, 2024 at 2:02 PM Leon Romanovsky <leon@kernel.org> wrote: > > On Tue, Jul 09, 2024 at 02:26:08PM -0700, Nathan Chancellor wrote: > > On Tue, Jul 09, 2024 at 03:27:37PM +0300, Leon Romanovsky wrote: > > > On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote: > > > > From: Peng Hao <flyingpeng@tencent.com> > > > > > > > > When building kernel with clang, which will typically > > > > have sanitizers enabled, there is a warning about a large stack frame. > > > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than] > > > > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer, > > > > ^ > > > > > > Please fix it, not hide it. > > > > Agreed, this is far from an acceptable solution. No details were > > provided around compiler, architecture, or configuration, so I can only > > speculate what is happening here. From reading the code, I suspect that > > ocrdma_add_stat() is getting inlined into all of its callsites but the > > stack slot for buff[128] is not getting reused, which may be related to > > a missing lifetime marker like [1] or sanitizer instrumentation. I am > > guessing that marking ocrdma_dbgfs_ops_read() as noinline_for_stack > > would resolve this. > > KASAN is generally enabled for testing environments; noinline_for_stack is used to solve the problem of needing to track deeper call chains. I also tried to use noinline_for_stack and issued a new patch, but I think the v1 method is better. My environment is: x86_64, clang version 15.0.7 CONFIG_FRAME_WARN=8192 > > static noinline_for_stack int ocrdma_add_stat(char *start, char *pcur, > > This is a good solution, but first let's see if the author can provide > more details. > > > > > If this is not tolerable for all configurations, it could be made more > > pointed with something like > > > > static > > #if defined(CONFIG_CC_IS_CLANG) && (defined(CONFIG_KASAN) || defined(CONFIG_KCSAN)) > > noinline_for_stack > > #endif > > int ocrdma_add_stat(char *start, char *pcur, > > > > but I am aware that is quite ugly. > > > > [1]: https://github.com/llvm/llvm-project/issues/38157#issuecomment-1756321571 > > > > Cheers, > > Nathan > > > > > > Increase the frame size by 20% to set. > > > > > > > > Signed-off-by: Peng Hao <flyingpeng@tencent.com> > > > > --- > > > > drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile > > > > index 14fba95021d8..a1e9fcc04751 100644 > > > > --- a/drivers/infiniband/hw/ocrdma/Makefile > > > > +++ b/drivers/infiniband/hw/ocrdma/Makefile > > > > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet > > > > > > > > obj-$(CONFIG_INFINIBAND_OCRDMA) += ocrdma.o > > > > > > > > +ifneq ($(CONFIG_FRAME_WARN),0) > > > > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) > > > > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664 > > > > +endif > > > > +endif > > > > + > > > > ocrdma-y := ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o > > > > -- > > > > 2.27.0 > > > > > > > >
On Wed, Jul 10, 2024 at 09:46:27PM +0800, Hao Peng wrote: > On Wed, Jul 10, 2024 at 2:02 PM Leon Romanovsky <leon@kernel.org> wrote: > > > > On Tue, Jul 09, 2024 at 02:26:08PM -0700, Nathan Chancellor wrote: > > > On Tue, Jul 09, 2024 at 03:27:37PM +0300, Leon Romanovsky wrote: > > > > On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote: > > > > > From: Peng Hao <flyingpeng@tencent.com> > > > > > > > > > > When building kernel with clang, which will typically > > > > > have sanitizers enabled, there is a warning about a large stack frame. > > > > > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than] > > > > > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer, > > > > > ^ > > > > > > > > Please fix it, not hide it. > > > > > > Agreed, this is far from an acceptable solution. No details were > > > provided around compiler, architecture, or configuration, so I can only > > > speculate what is happening here. From reading the code, I suspect that > > > ocrdma_add_stat() is getting inlined into all of its callsites but the > > > stack slot for buff[128] is not getting reused, which may be related to > > > a missing lifetime marker like [1] or sanitizer instrumentation. I am > > > guessing that marking ocrdma_dbgfs_ops_read() as noinline_for_stack This should have been ocrdma_add_stat()... > > > would resolve this. > > > > KASAN is generally enabled for testing environments; > noinline_for_stack is used to solve the problem of needing to > track deeper call chains. I also tried to use noinline_for_stack and > issued a new patch, but I think the v1 method is better. As far as I understand it, this stack usage would be a problem at runtime if running without VMAP_STACK, so I am going to disagree. v2 gets more at the heart of the problem (although I wonder if that just moves the high stack usage to those other functions under the limit). > My environment is: > x86_64, clang version 15.0.7 > CONFIG_FRAME_WARN=8192 Is CONFIG_KASAN_STACK enabled? I realized that could be another reason you see this warning, as that option is known to have high stack usage with clang. > > > static noinline_for_stack int ocrdma_add_stat(char *start, char *pcur, > > > > This is a good solution, but first let's see if the author can provide > > more details. > > > > > > > > If this is not tolerable for all configurations, it could be made more > > > pointed with something like > > > > > > static > > > #if defined(CONFIG_CC_IS_CLANG) && (defined(CONFIG_KASAN) || defined(CONFIG_KCSAN)) > > > noinline_for_stack > > > #endif > > > int ocrdma_add_stat(char *start, char *pcur, > > > > > > but I am aware that is quite ugly. > > > > > > [1]: https://github.com/llvm/llvm-project/issues/38157#issuecomment-1756321571 > > > > > > Cheers, > > > Nathan > > > > > > > > Increase the frame size by 20% to set. > > > > > > > > > > Signed-off-by: Peng Hao <flyingpeng@tencent.com> > > > > > --- > > > > > drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile > > > > > index 14fba95021d8..a1e9fcc04751 100644 > > > > > --- a/drivers/infiniband/hw/ocrdma/Makefile > > > > > +++ b/drivers/infiniband/hw/ocrdma/Makefile > > > > > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet > > > > > > > > > > obj-$(CONFIG_INFINIBAND_OCRDMA) += ocrdma.o > > > > > > > > > > +ifneq ($(CONFIG_FRAME_WARN),0) > > > > > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) > > > > > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664 > > > > > +endif > > > > > +endif > > > > > + > > > > > ocrdma-y := ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o > > > > > -- > > > > > 2.27.0 > > > > > > > > > >
diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile index 14fba95021d8..a1e9fcc04751 100644 --- a/drivers/infiniband/hw/ocrdma/Makefile +++ b/drivers/infiniband/hw/ocrdma/Makefile @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet obj-$(CONFIG_INFINIBAND_OCRDMA) += ocrdma.o +ifneq ($(CONFIG_FRAME_WARN),0) +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y) +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664 +endif +endif + ocrdma-y := ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o