All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brendan Higgins <brendanhiggins@google.com>
To: Shuah Khan <skhan@linuxfoundation.org>, Arnd Bergmann <arnd@arndb.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Naresh Kamboju <naresh.kamboju@linaro.org>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Ariel Elior <aelior@marvell.com>,
	GR-everest-linux-l2@marvell.com, Wei Liu <wei.liu@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	open list <linux-kernel@vger.kernel.org>,
	Netdev <netdev@vger.kernel.org>,
	lkft-triage@lists.linaro.org,
	"David S. Miller" <davem@davemloft.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	KUnit Development <kunit-dev@googlegroups.com>
Subject: Re: ipv4/tcp.c:4234:1: error: the frame size of 1152 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Date: Wed, 8 Sep 2021 14:24:50 -0700	[thread overview]
Message-ID: <CAFd5g47MgGCoenw08hehegstQSujT7AwksQkxA7mQgKhChimNw@mail.gmail.com> (raw)
In-Reply-To: <120389b9-f90b-0fa3-21d5-1f789b4c984d@linuxfoundation.org>

On Wed, Sep 8, 2021 at 10:16 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 9/8/21 11:05 AM, Arnd Bergmann wrote:
> > On Wed, Sep 8, 2021 at 4:12 PM Shuah Khan <skhan@linuxfoundation.org> wrote:
> >> On 9/7/21 5:14 PM, Linus Torvalds wrote:
> >>> The KUNIT macros create all these individually reasonably small
> >>> initialized structures on stack, and when you have more than a small
> >>> handful of them the KUNIT infrastructure just makes the stack space
> >>> explode. Sometimes the compiler will be able to re-use the stack
> >>> slots, but it seems to be an iffy proposition to depend on it - it
> >>> seems to be a combination of luck and various config options.
> >>>
> >>
> >> I have been concerned about these macros creeping in for a while.
> >> I will take a closer look and work with Brendan to come with a plan
> >> to address it.
> >
> > I've previously sent patches to turn off the structleak plugin for
> > any kunit test file to work around this, but only a few of those patches
> > got merged and new files have been added since. It would
> > definitely help to come up with a proper fix, but my structleak-disable
> > hack should be sufficient as a quick fix.
> >
>
> Looks like these are RFC patches and the discussion went cold. Let's pick
> this back up and we can make progress.
>
> https://lore.kernel.org/lkml/CAFd5g45+JqKDqewqz2oZtnphA-_0w62FdSTkRs43K_NJUgnLBg@mail.gmail.com/

I can try to get the patch reapplying and send it out (I just figured
that Arnd or Kees would want to send it out :-)  since it was your
idea).

I definitely agree that in the cases where KUnit is not actually
contributing to blowing the stack - struct leak just thinks it is,
this is fine; however, it sounds like Linus' concerns with KUnit's
macros go deeper than this. Arnd, I think you sketched out a way to
make the KUNIT_* macros take up less space, but after some
investigation we found that it was pretty inflexible.

Ideally test cases should never get big enough for KUNIT_* macros to
be a problem (when they do it is usually an indication that your test
case is trying to do too many things); nevertheless, we are still in
this situation.

I think I will need to dust off some cobwebs out of my brain to
remember why I didn't like the idea of making the KUNIT_* macros take
up less stack space.

WARNING: multiple messages have this Message-ID (diff)
From: Brendan Higgins <brendanhiggins@google.com>
To: Shuah Khan <skhan@linuxfoundation.org>, Arnd Bergmann <arnd@arndb.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	 Naresh Kamboju <naresh.kamboju@linaro.org>,
	Mathias Nyman <mathias.nyman@intel.com>,
	 Johannes Berg <johannes@sipsolutions.net>,
	Jakub Kicinski <kuba@kernel.org>,
	 Ariel Elior <aelior@marvell.com>,
	GR-everest-linux-l2@marvell.com,  Wei Liu <wei.liu@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	open list <linux-kernel@vger.kernel.org>,
	Netdev <netdev@vger.kernel.org>,
	 lkft-triage@lists.linaro.org,
	"David S. Miller" <davem@davemloft.net>,
	 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	 Nathan Chancellor <nathan@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	 Alexei Starovoitov <ast@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	 KUnit Development <kunit-dev@googlegroups.com>
Subject: Re: ipv4/tcp.c:4234:1: error: the frame size of 1152 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Date: Wed, 8 Sep 2021 14:24:50 -0700	[thread overview]
Message-ID: <CAFd5g47MgGCoenw08hehegstQSujT7AwksQkxA7mQgKhChimNw@mail.gmail.com> (raw)
In-Reply-To: <120389b9-f90b-0fa3-21d5-1f789b4c984d@linuxfoundation.org>

On Wed, Sep 8, 2021 at 10:16 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 9/8/21 11:05 AM, Arnd Bergmann wrote:
> > On Wed, Sep 8, 2021 at 4:12 PM Shuah Khan <skhan@linuxfoundation.org> wrote:
> >> On 9/7/21 5:14 PM, Linus Torvalds wrote:
> >>> The KUNIT macros create all these individually reasonably small
> >>> initialized structures on stack, and when you have more than a small
> >>> handful of them the KUNIT infrastructure just makes the stack space
> >>> explode. Sometimes the compiler will be able to re-use the stack
> >>> slots, but it seems to be an iffy proposition to depend on it - it
> >>> seems to be a combination of luck and various config options.
> >>>
> >>
> >> I have been concerned about these macros creeping in for a while.
> >> I will take a closer look and work with Brendan to come with a plan
> >> to address it.
> >
> > I've previously sent patches to turn off the structleak plugin for
> > any kunit test file to work around this, but only a few of those patches
> > got merged and new files have been added since. It would
> > definitely help to come up with a proper fix, but my structleak-disable
> > hack should be sufficient as a quick fix.
> >
>
> Looks like these are RFC patches and the discussion went cold. Let's pick
> this back up and we can make progress.
>
> https://lore.kernel.org/lkml/CAFd5g45+JqKDqewqz2oZtnphA-_0w62FdSTkRs43K_NJUgnLBg@mail.gmail.com/

I can try to get the patch reapplying and send it out (I just figured
that Arnd or Kees would want to send it out :-)  since it was your
idea).

I definitely agree that in the cases where KUnit is not actually
contributing to blowing the stack - struct leak just thinks it is,
this is fine; however, it sounds like Linus' concerns with KUnit's
macros go deeper than this. Arnd, I think you sketched out a way to
make the KUNIT_* macros take up less space, but after some
investigation we found that it was pretty inflexible.

Ideally test cases should never get big enough for KUNIT_* macros to
be a problem (when they do it is usually an indication that your test
case is trying to do too many things); nevertheless, we are still in
this situation.

I think I will need to dust off some cobwebs out of my brain to
remember why I didn't like the idea of making the KUNIT_* macros take
up less stack space.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-09-08 21:25 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-06 12:27 ipv4/tcp.c:4234:1: error: the frame size of 1152 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] Naresh Kamboju
2021-09-06 12:27 ` Naresh Kamboju
2021-09-07 22:16 ` Linus Torvalds
2021-09-07 22:16   ` Linus Torvalds
2021-09-07 23:14   ` Linus Torvalds
2021-09-07 23:14     ` Linus Torvalds
2021-09-07 23:35     ` Nathan Chancellor
2021-09-07 23:35       ` Nathan Chancellor
2021-09-07 23:49       ` Linus Torvalds
2021-09-07 23:49         ` Linus Torvalds
2021-09-08  0:15         ` Nathan Chancellor
2021-09-08  0:15           ` Nathan Chancellor
2021-09-08  1:35           ` Linus Torvalds
2021-09-08  1:35             ` Linus Torvalds
2021-09-08  1:43             ` Linus Torvalds
2021-09-08  1:43               ` Linus Torvalds
2021-09-08 17:00               ` Arnd Bergmann
2021-09-08 17:00                 ` Arnd Bergmann
2021-09-08  7:09     ` Johannes Berg
2021-09-08  7:09       ` Johannes Berg
2021-09-08 10:03     ` Wei Liu
2021-09-08 10:03       ` Wei Liu
2021-09-08 14:51       ` David Laight
2021-09-08 14:51         ` David Laight
2021-09-08 15:23         ` Wei Liu
2021-09-08 15:23           ` Wei Liu
2021-09-08 16:05           ` David Laight
2021-09-08 16:05             ` David Laight
2021-09-08 15:59       ` Linus Torvalds
2021-09-08 15:59         ` Linus Torvalds
2021-09-08 16:12         ` Wei Liu
2021-09-08 16:12           ` Wei Liu
2021-09-08 13:48     ` Thorsten Glaser
2021-09-08 13:48       ` Thorsten Glaser
2021-09-08 14:50       ` Eric Dumazet
2021-09-08 14:50         ` Eric Dumazet
2021-09-08 15:48         ` Linus Torvalds
2021-09-08 15:48           ` Linus Torvalds
2021-09-08 16:56           ` Arnd Bergmann
2021-09-08 16:56             ` Arnd Bergmann
2021-09-08 14:11     ` Shuah Khan
2021-09-08 14:11       ` Shuah Khan
2021-09-08 17:05       ` Arnd Bergmann
2021-09-08 17:05         ` Arnd Bergmann
2021-09-08 17:16         ` Shuah Khan
2021-09-08 17:16           ` Shuah Khan
2021-09-08 21:24           ` Brendan Higgins [this message]
2021-09-08 21:24             ` Brendan Higgins
2021-09-08 22:27             ` Linus Torvalds
2021-09-08 22:27               ` Linus Torvalds
2021-09-13 20:55             ` Shuah Khan
2021-09-13 20:55               ` Shuah Khan
2021-09-14 20:46               ` Brendan Higgins
2021-09-14 20:46                 ` Brendan Higgins
2021-09-14 22:03                 ` Arnd Bergmann
2021-09-14 22:03                   ` Arnd Bergmann
2021-09-17  5:39                   ` Brendan Higgins
2021-09-17  5:39                     ` Brendan Higgins
2021-09-17  6:16                     ` Brendan Higgins
2021-09-17  6:16                       ` Brendan Higgins
2021-09-17  7:20                       ` Arnd Bergmann
2021-09-17  7:20                         ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFd5g47MgGCoenw08hehegstQSujT7AwksQkxA7mQgKhChimNw@mail.gmail.com \
    --to=brendanhiggins@google.com \
    --cc=GR-everest-linux-l2@marvell.com \
    --cc=aelior@marvell.com \
    --cc=arnd@arndb.de \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=johannes@sipsolutions.net \
    --cc=kuba@kernel.org \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkft-triage@lists.linaro.org \
    --cc=mathias.nyman@intel.com \
    --cc=naresh.kamboju@linaro.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=torvalds@linux-foundation.org \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.