Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CA+G9fYtFvJdtBknaDKR54HHMf4XsXKD4UD3qXkQ1KhgY19n3tw@mail.gmail.com>
 <CAHk-=wisUqoX5Njrnnpp0pDx+bxSAJdPxfgEUv82tZkvUqoN1w@mail.gmail.com>
 <CAHk-=whF9F89vsfH8E9TGc0tZA-yhzi2Di8wOtquNB5vRkFX5w@mail.gmail.com> <20210908100304.oknxj4v436sbg3nb@liuwe-devbox-debian-v2>
In-Reply-To: <20210908100304.oknxj4v436sbg3nb@liuwe-devbox-debian-v2>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 8 Sep 2021 08:59:36 -0700
Message-ID: <CAHk-=wgMyCaFMybdQRJYJLbbbv5S2UHsU3oQ+CBRyYkvjmR=hA@mail.gmail.com>
Subject: Re: ipv4/tcp.c:4234:1: error: the frame size of 1152 bytes is larger
 than 1024 bytes [-Werror=frame-larger-than=]
To:     Wei Liu <wei.liu@kernel.org>
Cc:     Naresh Kamboju <naresh.kamboju@linaro.org>,
        Mathias Nyman <mathias.nyman@intel.com>,
        Johannes Berg <johannes@sipsolutions.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Shuah Khan <skhan@linuxfoundation.org>,
        Brendan Higgins <brendanhiggins@google.com>,
        Ariel Elior <aelior@marvell.com>,
        GR-everest-linux-l2@marvell.com,
        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,
        Arnd Bergmann <arnd@arndb.de>,
        "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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Sep 8, 2021 at 3:03 AM Wei Liu <wei.liu@kernel.org> wrote:
>
> Thanks for the heads-up. I found one instance of this bad practice in
> hv_apic.c. Presumably that's the one you were referring to.

Yeah, that must have been the one I saw.

> However calling into the allocator from that IPI path seems very heavy
> weight. I will discuss with fellow engineers on how to fix it properly.

In other places, the options have been fairly straightforward:

 (a) avoid the allocation entirely.

I think the main reason hyperv does it is because it wants to clear
the "current cpu" from the cpumask for the ALL_BUT_SELF case, and if
you can just instead keep track of that "all but self" bit separately
and pass it down the call chain, you may not need that allocation at
all.

 (b) use a static percpu allocation

The IPI code generally wants interrupts disabled anyway, or it
certainly can just do the required preemption disable to make sure
that it has exclusive access to a percpu allocation.

 (c) take advantage of any hypervisor limitations

If hyperv is limited to some smaller number of CPU's - google seems to
imply 240 - maybe you can keep such a smaller allocation on the stack,
but just verify that the incoming cpumask is less than whatever the
hyperv limit is.

That (c) is obviously the worst choice in some sense, but in some
cases limiting yourself to simplify things isn't wrong.

I suspect the percpu allocation model is the easiest one. It's what
other IPI code does. See for example

  arch/x86/kernel/apic/x2apic_cluster.c:
      __x2apic_send_IPI_mask()

and that percpu 'ipi_mask' thing.

That said, if you are already just iterating over the mask, doing (a)
may be trivial. No allocation at all is even better than a percpu one.

I'm sure there are other options. The above is just the obvious ones
that come to mind.

           Linus