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, 6 Nov 2019 17:34:07 +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 v4 3/9] perf evlist: Maintain evlist->all_cpus
Message-ID: <20191106163407.GN30214@krava>
References: <20191105002522.83803-1-andi@firstfloor.org>
 <20191105002522.83803-4-andi@firstfloor.org>
MIME-Version: 1.0
In-Reply-To: <20191105002522.83803-4-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 Mon, Nov 04, 2019 at 04:25:16PM -0800, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>=20
> Maintain a cpumap in the evlist that is the union of all the cpus
> of the events.
>=20
> This needs a cpumap merge operation. To make the merge operation
> work efficiently maintain the cpumap in a sorted state.
>=20
> The sorting is also needed for the affinity loop changes later.
>=20
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/lib/cpumap.c                  | 42 ++++++++++++++++++++++++
>  tools/perf/lib/evlist.c                  |  1 +
>  tools/perf/lib/include/internal/evlist.h |  1 +
>  tools/perf/lib/include/perf/cpumap.h     |  2 ++
>  4 files changed, 46 insertions(+)
>=20
> diff --git a/tools/perf/lib/cpumap.c b/tools/perf/lib/cpumap.c
> index 2ca1fafa620d..5ca26a9d318f 100644
> --- a/tools/perf/lib/cpumap.c
> +++ b/tools/perf/lib/cpumap.c
> @@ -68,6 +68,11 @@ static struct perf_cpu_map *cpu_map__default_new(void)
>  =09return cpus;
>  }
> =20
> +static int cmp_int(const void *a, const void *b)
> +{
> +=09return *(const int *)a - *(const int*)b;
> +}
> +
>  static struct perf_cpu_map *cpu_map__trim_new(int nr_cpus, int *tmp_cpus=
)
>  {
>  =09size_t payload_size =3D nr_cpus * sizeof(int);
> @@ -76,6 +81,7 @@ static struct perf_cpu_map *cpu_map__trim_new(int nr_cp=
us, int *tmp_cpus)
>  =09if (cpus !=3D NULL) {
>  =09=09cpus->nr =3D nr_cpus;
>  =09=09memcpy(cpus->map, tmp_cpus, payload_size);
> +=09=09qsort(cpus->map, nr_cpus, sizeof(int), cmp_int);
>  =09=09refcount_set(&cpus->refcnt, 1);
>  =09}
=20
please move the sort into separate change

> =20
> @@ -272,3 +278,39 @@ int perf_cpu_map__max(struct perf_cpu_map *map)
> =20
>  =09return max;
>  }
> +
> +struct perf_cpu_map *perf_cpu_map__update(struct perf_cpu_map *orig,
> +=09=09=09=09=09  struct perf_cpu_map *other)
> +{

it's more like perf_cpu_map__merge, right?

> +=09int *tmp_cpus;
> +=09int tmp_len;
> +=09int i, j, k;
> +=09struct perf_cpu_map *merged;
> +
> +=09if (!orig) {
> +=09=09perf_cpu_map__get(other);
> +=09=09return other;
> +=09}
> +=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=09tmp_cpus[k++] =3D orig->map[i++];
> +=09=09else
> +=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 < tmp_len);
> +=09merged =3D cpu_map__trim_new(k, tmp_cpus);
> +=09free(tmp_cpus);
> +=09return merged;
> +}

this is great function for automated test, please add

thanks,
jirka