From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 76647C77B78 for ; Tue, 2 May 2023 20:37:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229720AbjEBUhX (ORCPT ); Tue, 2 May 2023 16:37:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59796 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229515AbjEBUhW (ORCPT ); Tue, 2 May 2023 16:37:22 -0400 Received: from mail-il1-x12f.google.com (mail-il1-x12f.google.com [IPv6:2607:f8b0:4864:20::12f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D028F19A2 for ; Tue, 2 May 2023 13:37:20 -0700 (PDT) Received: by mail-il1-x12f.google.com with SMTP id e9e14a558f8ab-3311833ba3dso156665ab.0 for ; Tue, 02 May 2023 13:37:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1683059840; x=1685651840; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=3Cl+EqDeYwpTNeDyJZ31cWxa2e7SIFMBY+Z+r+9CTeM=; b=RoE+6ObKk+Ofk7Ps/wn5WI10EqZLQwmhPRozbE2ZSakrN7Xaq0YaERAw0+rOd7Stoc 3ojazmjJ8k2LZwNBmsH/62gjYAF1gMzbrkvs5GwP/0ZuI0RjhR7PMmPZMq9jDqOzF4aq PccfjEK87WMbnLPlwOM0rckBIJbaduL2jC8q9KuFjMRti2ry+x40B7WmvgMwdIbqfhYn t4Wb25WZYvgbyKpq37O/s8HavFjMEHY8g+CyEykP8X+czykNQ41CJTO+NsFelff75ljl j/JnuBHoWW/0og7moMOKss3F9ExA8JVMUP2AL6iuFDnoUsFl7HiIladdEm2+DO0f75NF 6tww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683059840; x=1685651840; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3Cl+EqDeYwpTNeDyJZ31cWxa2e7SIFMBY+Z+r+9CTeM=; b=aI8Ggsv7SCUdFu0o4M++vPH7B7n+sqNv1nufjTIbcG03XXbvcUIr8P/sauqUxXXtKe aNZzR/yGrCmBlIhggh/f9oKo80j56fkf79XFbKFV0AXExgDSDOEL5IZF08Tysqmfwbow m9nTiQjlK+dGn+UHVNp6f1iDsmgmSltWPRVGu7IYEIH6hJsyLWYP84ApznYimsOhY5/p PbWqyUdsMz07iibSI9aq4cx6kzCoPrkE85bOIN6ivk3od/OduCevsz9OJBrt2OL+LUO0 NrqOQ0dOxLOeuplukXUloZ00AUn9pH0zPYOWULFySn7rsuqsuYLOYwsgdhKGOnsBmG7y N1Wg== X-Gm-Message-State: AC+VfDzFyUFMnQrIpk633eh0DU6qGdHGI8yNjB1p/xVZynnC/Jb8lIbO tojWeEhkWyVo4YK5GtWMDCUJuRJhnlizDByb0Y2BWw== X-Google-Smtp-Source: ACHHUZ75SsGaI9x5mLnHNfrvwr3lXTjJQxO+5cZDVkg8RLDJbVmEjlQf6e4l6ixWHvLHNMXWECbK55bHC57ZYECbiSY= X-Received: by 2002:a05:6e02:178d:b0:326:55d0:efad with SMTP id y13-20020a056e02178d00b0032655d0efadmr76821ilu.12.1683059839895; Tue, 02 May 2023 13:37:19 -0700 (PDT) MIME-Version: 1.0 References: <20230502203135.24794-1-jinli.xiao@nyu.edu> In-Reply-To: <20230502203135.24794-1-jinli.xiao@nyu.edu> From: Ian Rogers Date: Tue, 2 May 2023 13:37:04 -0700 Message-ID: Subject: Re: [PATCH] perf python: Set error messages on call failure To: Jinli Xiao Cc: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Adrian Hunter , Leo Yan , Suzuki Poulouse , James Clark , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org On Tue, May 2, 2023 at 1:31=E2=80=AFPM Jinli Xiao wrot= e: > > Updates the perf python binding to provide more informative error > messages to the user when there is a call failure. The changes include > setting error messages on several different scenarios when the package > needs to return -1 or NULL. > > Signed-off-by: Jinli Xiao Nice! Would it be possible to test this? We could do a shell test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/too= ls/perf/tests/shell Thanks, Ian > --- > tools/perf/util/python.c | 61 ++++++++++++++++++++++++++++++---------- > 1 file changed, 46 insertions(+), 15 deletions(-) > > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c > index 42e8b813d010..7e6b12e87744 100644 > --- a/tools/perf/util/python.c > +++ b/tools/perf/util/python.c > @@ -635,12 +635,16 @@ static int pyrf_cpu_map__init(struct pyrf_cpu_map *= pcpus, > char *cpustr =3D NULL; > > if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|s", > - kwlist, &cpustr)) > + kwlist, &cpustr)) { > + PyErr_BadArgument(); > return -1; > + } > > pcpus->cpus =3D perf_cpu_map__new(cpustr); > - if (pcpus->cpus =3D=3D NULL) > + if (pcpus->cpus =3D=3D NULL) { > + PyErr_SetFromErrno(PyExc_OSError); > return -1; > + } > return 0; > } > > @@ -704,12 +708,16 @@ static int pyrf_thread_map__init(struct pyrf_thread= _map *pthreads, > int pid =3D -1, tid =3D -1, uid =3D UINT_MAX; > > if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|iii", > - kwlist, &pid, &tid, &uid)) > + kwlist, &pid, &tid, &uid)) { > + PyErr_BadArgument(); > return -1; > + } > > pthreads->threads =3D thread_map__new(pid, tid, uid); > - if (pthreads->threads =3D=3D NULL) > + if (pthreads->threads =3D=3D NULL) { > + PyErr_SetFromErrno(PyExc_OSError); > return -1; > + } > return 0; > } > > @@ -839,13 +847,18 @@ static int pyrf_evsel__init(struct pyrf_evsel *pevs= el, > &enable_on_exec, &task, &waterma= rk, > &precise_ip, &mmap_data, &sample= _id_all, > &attr.wakeup_events, &attr.bp_ty= pe, > - &attr.bp_addr, &attr.bp_len, &id= x)) > + &attr.bp_addr, &attr.bp_len, &id= x)) { > + PyErr_BadArgument(); > return -1; > + } > > /* union... */ > if (sample_period !=3D 0) { > - if (attr.sample_freq !=3D 0) > - return -1; /* FIXME: throw right exception */ > + if (attr.sample_freq !=3D 0) { > + PyErr_SetString(PyExc_ValueError, > + "perf: sample_freq and sample_per= iod are mutually exclusive"); > + return -1; > + } > attr.sample_period =3D sample_period; > } > > @@ -892,8 +905,10 @@ static PyObject *pyrf_evsel__open(struct pyrf_evsel = *pevsel, > static char *kwlist[] =3D { "cpus", "threads", "group", "inherit"= , NULL }; > > if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOii", kwlist, > - &pcpus, &pthreads, &group, &inhe= rit)) > + &pcpus, &pthreads, &group, &inhe= rit)) { > + PyErr_BadArgument(); > return NULL; > + } > > if (pthreads !=3D NULL) > threads =3D ((struct pyrf_thread_map *)pthreads)->threads= ; > @@ -957,8 +972,10 @@ static int pyrf_evlist__init(struct pyrf_evlist *pev= list, > struct perf_cpu_map *cpus; > struct perf_thread_map *threads; > > - if (!PyArg_ParseTuple(args, "OO", &pcpus, &pthreads)) > + if (!PyArg_ParseTuple(args, "OO", &pcpus, &pthreads)) { > + PyErr_BadArgument(); > return -1; > + } > > threads =3D ((struct pyrf_thread_map *)pthreads)->threads; > cpus =3D ((struct pyrf_cpu_map *)pcpus)->cpus; > @@ -980,8 +997,10 @@ static PyObject *pyrf_evlist__mmap(struct pyrf_evlis= t *pevlist, > int pages =3D 128, overwrite =3D false; > > if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ii", kwlist, > - &pages, &overwrite)) > + &pages, &overwrite)) { > + PyErr_BadArgument(); > return NULL; > + } > > if (evlist__mmap(evlist, pages) < 0) { > PyErr_SetFromErrno(PyExc_OSError); > @@ -999,8 +1018,10 @@ static PyObject *pyrf_evlist__poll(struct pyrf_evli= st *pevlist, > static char *kwlist[] =3D { "timeout", NULL }; > int timeout =3D -1, n; > > - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|i", kwlist, &tim= eout)) > + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|i", kwlist, &tim= eout)) { > + PyErr_BadArgument(); > return NULL; > + } > > n =3D evlist__poll(evlist, timeout); > if (n < 0) { > @@ -1057,8 +1078,10 @@ static PyObject *pyrf_evlist__add(struct pyrf_evli= st *pevlist, > PyObject *pevsel; > struct evsel *evsel; > > - if (!PyArg_ParseTuple(args, "O", &pevsel)) > + if (!PyArg_ParseTuple(args, "O", &pevsel)) { > + PyErr_BadArgument(); > return NULL; > + } > > Py_INCREF(pevsel); > evsel =3D &((struct pyrf_evsel *)pevsel)->evsel; > @@ -1093,12 +1116,16 @@ static PyObject *pyrf_evlist__read_on_cpu(struct = pyrf_evlist *pevlist, > int err; > > if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|i", kwlist, > - &cpu, &sample_id_all)) > + &cpu, &sample_id_all)) { > + PyErr_BadArgument(); > return NULL; > + } > > md =3D get_md(evlist, cpu); > - if (!md) > + if (!md) { > + PyErr_SetFromErrno(PyExc_OSError); > return NULL; > + } > > if (perf_mmap__read_init(&md->core) < 0) > goto end; > @@ -1324,6 +1351,8 @@ static PyObject *pyrf__tracepoint(struct pyrf_evsel= *pevsel, > PyObject *args, PyObject *kwargs) > { > #ifndef HAVE_LIBTRACEEVENT > + PyErr_SetString(PyExc_OSError, > + "perf: tracepoint support not compiled in"); > return NULL; > #else > struct tep_event *tp_format; > @@ -1332,8 +1361,10 @@ static PyObject *pyrf__tracepoint(struct pyrf_evse= l *pevsel, > char *name =3D NULL; > > if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ss", kwlist, > - &sys, &name)) > + &sys, &name)) { > + PyErr_BadArgument(); > return NULL; > + } > > tp_format =3D trace_event__tp_format(sys, name); > if (IS_ERR(tp_format)) > -- > 2.39.2 >