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;
Date:   Wed, 20 Nov 2019 16:07:32 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Andi Kleen <andi@firstfloor.org>
Cc:     acme@kernel.org, jolsa@kernel.org, linux-kernel@vger.kernel.org,
        Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH v7 09/12] perf stat: Use affinity for opening events
Message-ID: <20191120150732.GE4007@krava>
References: <20191116055229.62002-1-andi@firstfloor.org>
 <20191116055229.62002-10-andi@firstfloor.org>
MIME-Version: 1.0
In-Reply-To: <20191116055229.62002-10-andi@firstfloor.org>
User-Agent: Mutt/1.12.1 (2019-06-15)
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Nov 15, 2019 at 09:52:26PM -0800, Andi Kleen wrote:

SNIP

> +=09if (second_pass) {
> +=09=09/*
> +=09=09 * Now redo all the weak group after closing them,
> +=09=09 * and also close errored counters.
> +=09=09 */
> +
> +=09=09evlist__for_each_cpu (evsel_list, i, cpu) {
> +=09=09=09affinity__set(&affinity, cpu);
> +=09=09=09/* First close errored or weak retry */
> +=09=09=09evlist__for_each_entry(evsel_list, counter) {
> +=09=09=09=09if (!counter->reset_group && !counter->errored)
> +=09=09=09=09=09continue;
> +=09=09=09=09if (evsel__cpu_iter_skip_no_inc(counter, cpu))
> +=09=09=09=09=09continue;
> +=09=09=09=09perf_evsel__close_cpu(&counter->core, counter->cpu_iter);
> +=09=09=09}
> +=09=09=09/* Now reopen weak */
> +=09=09=09evlist__for_each_entry(evsel_list, counter) {
> +=09=09=09=09if (!counter->reset_group && !counter->errored)
> +=09=09=09=09=09continue;
> +=09=09=09=09if (evsel__cpu_iter_skip(counter, cpu))
> +=09=09=09=09=09continue;

why staring at this I wonder why can't we call perf_evsel__close_cpu in
here and remove the above loop? together with evsel__cpu_iter_skip_no_inc
function

jirka


> +=09=09=09=09if (!counter->reset_group)
> +=09=09=09=09=09continue;
> +try_again_reset:
> +=09=09=09=09pr_debug2("reopening weak %s\n", perf_evsel__name(counter));
> +=09=09=09=09if (create_perf_stat_counter(counter, &stat_config, &target,
> +=09=09=09=09=09=09=09     counter->cpu_iter - 1) < 0) {
> +
> +=09=09=09=09=09switch (stat_handle_error(counter)) {
> +=09=09=09=09=09case COUNTER_FATAL:
> +=09=09=09=09=09=09return -1;
> +=09=09=09=09=09case COUNTER_RETRY:
> +=09=09=09=09=09=09goto try_again_reset;
> +=09=09=09=09=09case COUNTER_SKIP:
> +=09=09=09=09=09=09continue;
> +=09=09=09=09=09default:
> +=09=09=09=09=09=09break;
> +=09=09=09=09=09}
> +=09=09=09=09}
> +=09=09=09=09counter->supported =3D true;
>  =09=09=09}
>  =09=09}
> -=09=09counter->supported =3D true;
> +=09}

SNIP