On Wed, Feb 13, 2019 at 5:13 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > There is this queued: > > http://lkml.kernel.org/r/tip-528871b456026e6127d95b1b2bd8e3a003dc1614@xxxxxxxxxxxxxx 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
