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:   Mon, 11 Nov 2019 14:31:37 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Andi Kleen <andi@firstfloor.org>
Cc:     jolsa@kernel.org, acme@kernel.org, linux-kernel@vger.kernel.org,
        Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH v5 04/13] perf evlist: Maintain evlist->all_cpus
Message-ID: <20191111133137.GH12923@krava>
References: <20191107181646.506734-1-andi@firstfloor.org>
 <20191107181646.506734-5-andi@firstfloor.org>
MIME-Version: 1.0
In-Reply-To: <20191107181646.506734-5-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 Thu, Nov 07, 2019 at 10:16:37AM -0800, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/lib/cpumap.c b/tools/perf/lib/cpumap.c
> index d81656b4635e..b9f573438b93 100644
> --- a/tools/perf/lib/cpumap.c
> +++ b/tools/perf/lib/cpumap.c
> @@ -286,3 +286,46 @@ int perf_cpu_map__max(struct perf_cpu_map *map)
> =20
>  =09return max;
>  }
> +
> +struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
> +=09=09=09=09=09 struct perf_cpu_map *other)
> +{
> +=09int *tmp_cpus;
> +=09int tmp_len;
> +=09int i, j, k;
> +=09struct perf_cpu_map *merged;
> +
> +=09if (!orig && !other)
> +=09=09return NULL;
> +=09if (!orig) {
> +=09=09perf_cpu_map__get(other);
> +=09=09return other;
> +=09}
> +=09if (!other)
> +=09=09return orig;

so we bump refcnt for other but not for orig?

could you please put to the comment expected results
wrt refcnt values for above cases?

> +=09if (orig->nr =3D=3D other->nr &&
> +=09    !memcmp(orig->map, other->map, orig->nr * sizeof(int)))
> +=09=09return orig;
> +=09tmp_len =3D orig->nr + other->nr;
> +=09tmp_cpus =3D malloc(tmp_len * sizeof(int));
> +=09if (!tmp_cpus)
> +=09=09return NULL;
> +=09i =3D j =3D k =3D 0;
> +=09while (i < orig->nr && j < other->nr) {
> +=09=09if (orig->map[i] <=3D other->map[j]) {
> +=09=09=09if (orig->map[i] =3D=3D other->map[j])
> +=09=09=09=09j++;
> +=09=09=09tmp_cpus[k++] =3D orig->map[i++];
> +=09=09} else
> +=09=09=09tmp_cpus[k++] =3D other->map[j++];
> +=09}
> +=09while (i < orig->nr)
> +=09=09tmp_cpus[k++] =3D orig->map[i++];
> +=09while (j < other->nr)
> +=09=09tmp_cpus[k++] =3D other->map[j++];
> +=09assert(k <=3D tmp_len);
> +=09merged =3D cpu_map__trim_new(k, tmp_cpus);
> +=09free(tmp_cpus);
> +=09perf_cpu_map__put(orig);
> +=09return merged;

please use some comments and blank lines to separate
the code above, it's too much.. ;-)

SNIP

>  LIBPERF_API int perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int i=
dx);
>  LIBPERF_API int perf_cpu_map__nr(const struct perf_cpu_map *cpus);
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-t=
est.c
> index 8b286e9b7549..5fa37cf7f283 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -259,6 +259,11 @@ static struct test generic_tests[] =3D {
>  =09=09.desc =3D "Print cpu map",
>  =09=09.func =3D test__cpu_map_print,
>  =09},
> +=09{
> +=09=09.desc =3D "Merge cpu map",
> +=09=09.func =3D test__cpu_map_merge,
> +=09},

awesome ,thanks

jirka