From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755009AbbG1Fsa (ORCPT ); Tue, 28 Jul 2015 01:48:30 -0400 Received: from mga11.intel.com ([192.55.52.93]:64477 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751709AbbG1Fs2 (ORCPT ); Tue, 28 Jul 2015 01:48:28 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,560,1432623600"; d="scan'208";a="736898966" From: Alexander Shishkin To: Mathieu Poirier , a.p.zijlstra@chello.nl, mingo@redhat.com, acme@kernel.org Cc: linux-kernel@vger.kernel.org, mathieu.poirier@linaro.org Subject: Re: [PATCH] perf: fixing variable initialisation In-Reply-To: <1438036879-16340-1-git-send-email-mathieu.poirier@linaro.org> References: <1438036879-16340-1-git-send-email-mathieu.poirier@linaro.org> User-Agent: Notmuch/0.20.2 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Tue, 28 Jul 2015 08:48:25 +0300 Message-ID: <87k2tkbzo6.fsf@ashishki-desk.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mathieu Poirier writes: > Variable 'max_order' needs to be initialised to log2 of the > number of pages invariant of the no scather-gather PMU option. > Otherwise 'rb_alloc_aux_page' gets an order that is equal to > '0', which prevents the private page information to be set > properly. But that wouldn't make sense: we're only using the private field in pages at the bottom of high-order allocations, which only ever happen for NO_SG devices. Otherwise, every page is the buffer is just an individual page and doesn't need anything in the private field. > Signed-off-by: Mathieu Poirier > --- > kernel/events/ring_buffer.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c > index 96472824a752..bd3da7bebe9e 100644 > --- a/kernel/events/ring_buffer.c > +++ b/kernel/events/ring_buffer.c > @@ -449,18 +449,18 @@ int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event, > { > bool overwrite = !(flags & RING_BUFFER_WRITABLE); > int node = (event->cpu == -1) ? -1 : cpu_to_node(event->cpu); > - int ret = -ENOMEM, max_order = 0; > + int ret = -ENOMEM, max_order; > > if (!has_aux(event)) > return -ENOTSUPP; > > - if (event->pmu->capabilities & PERF_PMU_CAP_AUX_NO_SG) { > - /* > - * We need to start with the max_order that fits in nr_pages, > - * not the other way around, hence ilog2() and not get_order. > - */ > - max_order = ilog2(nr_pages); > + /* > + * We need to start with the max_order that fits in nr_pages, > + * not the other way around, hence ilog2() and not get_order. > + */ > + max_order = ilog2(nr_pages); Also, nr_pages here is the whole buffer, but when we do use page::private, we put an each individual high-order allocation's order there, not the whole buffer's, which makes sense because you'd need to know the former to program the HW properly, whereas the latter you'd get as a parameter to pmu::setup_aux() callback anyway. Regards, -- Alex