Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190211141827.214852402@linuxfoundation.org> <20190211141841.367111122@linuxfoundation.org>
 <1eb319822aa9f75f423a7790e78315e7672036b9.camel@nokia.com> <20190213131333.GT32494@hirez.programming.kicks-ass.net>
In-Reply-To: <20190213131333.GT32494@hirez.programming.kicks-ass.net>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 13 Feb 2019 10:08:35 -0800
Message-ID: <CAHk-=wgpU+5zAgakcDksXfEdJCtBFRdHm7Hqb_iDc+VWYKiqnA@mail.gmail.com>
Subject: Re: [PATCH 4.14 198/205] perf/core: Dont WARN() for impossible
 ring-buffer sizes
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     "Rantala, Tommi T. (Nokia - FI/Espoo)" <tommi.t.rantala@nokia.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "mingo@kernel.org" <mingo@kernel.org>,
        "jolsa@redhat.com" <jolsa@redhat.com>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>,
        "namhyung@kernel.org" <namhyung@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "acme@redhat.com" <acme@redhat.com>,
        "alexander.shishkin@linux.intel.com" 
        <alexander.shishkin@linux.intel.com>,
        "julien.thierry@arm.com" <julien.thierry@arm.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Feb 13, 2019 at 5:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> There is this queued:
>
>   http://lkml.kernel.org/r/tip-528871b456026e6127d95b1b2bd8e3a003dc1614@git.kernel.org

I think it would likely have been better to just remove the test
entirely, and instead just use __GFP_NOWARN.

This is an allocation essentially done for the user (as part of
mmap(), and we handle the failure case fine. So it doesn't matter
whether the warning is due to being out of memory or being a bad size,
we shouldn't cause a kernel warning.

That said, I think we should *indepdendently* have some sane limit for
"nr_pages", simply because we do that

        ...
        for (i = 0; i < nr_pages; i++) {
                rb->data_pages[i] = perf_mmap_alloc_page(cpu);

thing, which is really really expensive when "nr_pages" is in the
thousands (or millions). The fact that we end up almost _accidentally_
limiting nr_pages simply because the kzmalloc() size is limited is I
think a mistake.

So I'd rather see __GFP_NORWARN _and_ some kind of "don't allow crazy
perf ring buffers" check that is separate and independent from any
kmalloc issue..

For example, right now the maximum size you can allocate is (insanely)
tied to the page size. With a bigger page size, not only do you get a
bigger ring buffer anyway, but you also get more 'nr_pages', so it's
basically quadratic behavior. Does that make sense? It doesn't sound
that way to me.

                  Linus