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 X-Spam-Level: X-Spam-Status: No, score=-11.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05D45C433ED for ; Wed, 31 Mar 2021 22:07:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C7EA461029 for ; Wed, 31 Mar 2021 22:07:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232801AbhCaWHN (ORCPT ); Wed, 31 Mar 2021 18:07:13 -0400 Received: from mail.kernel.org ([198.145.29.99]:45314 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232655AbhCaWHB (ORCPT ); Wed, 31 Mar 2021 18:07:01 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 7E6656108D for ; Wed, 31 Mar 2021 22:07:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1617228420; bh=ewnEV2u+9OfHU1RtVtv1pWSjFZhmQ1lVFRdpH4p+CNI=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=jnfHAJ/P2KKzZKxod/gPFNRyZ38N8tTDBf9vH6CzCMuFauFnmsx5kWadY9+FcrIwI xOGnG/yG5xgIIo2vofd0t9d5a54ao+BmR9iY5ufYIUqYPAnOzWaddso3KJAIK/2psx TXViI1UUpvC5AXpnYYQqV4OFC7XLrjI3NR+9J8ZLMcbazuggqDHdYCMP9mIuzYOjOJ ZVYI+uzAWJCevzECkMJLgs4ABQ+eAKSBgwaXKsQ9Pl97lZnqpQlv1jiV/CeBPCZo44 +KnncrSpa+PevYkW8QDVpH/h69uhk36YAbFX837puR7QEY2D4MukNWQXIZI7Q1+K0u 78bfbub1QKtUw== Received: by mail-ej1-f47.google.com with SMTP id kt15so32293997ejb.12 for ; Wed, 31 Mar 2021 15:07:00 -0700 (PDT) X-Gm-Message-State: AOAM533uq9XZIf6Sz6UAkpoDzXtGJmYW+rvR2Zq8W80stsgH85xCq6sP hKbYiV1X7L31pYnppOrF2BMkotN951wxe8R7dg== X-Google-Smtp-Source: ABdhPJwrN3REAzz1OFnLicmVPUk5n1+yxRC0vvnXdJFX4tBIzONHIoaDA5yrbSrYr/JFf8dREgK9I8MOVvtnzjaBbz4= X-Received: by 2002:a17:906:2312:: with SMTP id l18mr6123769eja.468.1617228418909; Wed, 31 Mar 2021 15:06:58 -0700 (PDT) MIME-Version: 1.0 References: <20210311000837.3630499-1-robh@kernel.org> <20210311000837.3630499-5-robh@kernel.org> In-Reply-To: From: Rob Herring Date: Wed, 31 Mar 2021 17:06:46 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v6 04/10] libperf: Add evsel mmap support To: Jiri Olsa Cc: Will Deacon , Catalin Marinas , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Ian Rogers , Alexander Shishkin , Honnappa Nagarahalli , Zachary.Leaf@arm.com, Raphael Gault , Jonathan Cameron , Namhyung Kim , Itaru Kitayama , linux-arm-kernel , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 12, 2021 at 12:29 PM Jiri Olsa wrote: > > On Fri, Mar 12, 2021 at 07:34:39AM -0700, Rob Herring wrote: > > On Fri, Mar 12, 2021 at 6:59 AM Jiri Olsa wrote: > > > > > > On Wed, Mar 10, 2021 at 05:08:31PM -0700, Rob Herring wrote: > > > > > > SNIP > > > > > > > + > > > > static int > > > > sys_perf_event_open(struct perf_event_attr *attr, > > > > pid_t pid, int cpu, int group_fd, > > > > @@ -137,6 +147,8 @@ void perf_evsel__free_fd(struct perf_evsel *evsel) > > > > { > > > > xyarray__delete(evsel->fd); > > > > evsel->fd = NULL; > > > > + xyarray__delete(evsel->mmap); > > > > + evsel->mmap = NULL; > > > > } > > > > > > > > void perf_evsel__close(struct perf_evsel *evsel) > > > > @@ -156,6 +168,45 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu) > > > > perf_evsel__close_fd_cpu(evsel, cpu); > > > > } > > > > > > > > +int perf_evsel__mmap(struct perf_evsel *evsel, int pages) > > > > +{ > > > > + int ret, cpu, thread; > > > > + struct perf_mmap_param mp = { > > > > + .prot = PROT_READ | PROT_WRITE, > > > > + .mask = (pages * page_size) - 1, > > > > + }; > > > > > > I don't mind using evsel->fd for dimensions below, > > > but we need to check in here that it's defined, > > > that perf_evsel__open was called > > > > Right, so I'll add this here: > > > > if (evsel->fd == NULL) > > return -EINVAL; Actually, pretty much none of the API checks this. perf_evsel__run_ioctl(), perf_evsel__enable(), perf_evsel__disable(), perf_evsel__read() will all just deference evsel->fd. So fix all of these or follow existing behavior? > > Note that struct evsel has dimensions in it, but they are only set in > > the evlist code. I couldn't tell if that was by design or mistake. > > we do? we have the cpus and threads in perf_evsel no? Right, perf_evsel has cpus and threads pointers which in turn have the sizes, but those pointers are not set within the evsel code. > also you'd need perf_evsel not evsel, right? > > > > > BTW, I just noticed perf_evsel__open is leaking memory on most of its > > error paths. > > nice.. let's fix it ;-) NM, I missed that they are static... Rob 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 X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6BB01C433ED for ; Wed, 31 Mar 2021 22:09:02 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0C9EB61005 for ; Wed, 31 Mar 2021 22:09:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0C9EB61005 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=xr/yok/Acm3dPCQyz1jpOh8oxAAp4S4g1jybx1OsMYE=; b=DfDeoUUij/QRhfYr2cXzneeHE ZAPJwDaQn1G/86m0Q5NIJCvtYEG+XivQ+i/TO/x4HOzILGFe47MAeykrqTq4AnNgMNhCDVor+8Z2O 30qyO95+aooMjMZaDRfC02Uvcxw2X0ZLmeXsuJ6ePBgcmCZBwGqzt3yhhEna/suR0YfDYtOigfmJl KfusME0rc7z4b6OqZ53nYdOcD81Ny1bTSENKFIca/pnG+3DKJui/9GYt6zYssLZCWbCWlENu4bLw3 xDjCJLMXdCur+c+o+SK8l1o4+KcfoqYbALPHvxWhwiJT/6GgcMhBdN4af1uNyjROqZZSvwG1Bmuv/ 9dIcB873Q==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lRiza-007nON-7S; Wed, 31 Mar 2021 22:07:14 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lRizP-007nNE-Md for linux-arm-kernel@lists.infradead.org; Wed, 31 Mar 2021 22:07:06 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 74DDE60551 for ; Wed, 31 Mar 2021 22:07:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1617228420; bh=ewnEV2u+9OfHU1RtVtv1pWSjFZhmQ1lVFRdpH4p+CNI=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=jnfHAJ/P2KKzZKxod/gPFNRyZ38N8tTDBf9vH6CzCMuFauFnmsx5kWadY9+FcrIwI xOGnG/yG5xgIIo2vofd0t9d5a54ao+BmR9iY5ufYIUqYPAnOzWaddso3KJAIK/2psx TXViI1UUpvC5AXpnYYQqV4OFC7XLrjI3NR+9J8ZLMcbazuggqDHdYCMP9mIuzYOjOJ ZVYI+uzAWJCevzECkMJLgs4ABQ+eAKSBgwaXKsQ9Pl97lZnqpQlv1jiV/CeBPCZo44 +KnncrSpa+PevYkW8QDVpH/h69uhk36YAbFX837puR7QEY2D4MukNWQXIZI7Q1+K0u 78bfbub1QKtUw== Received: by mail-ej1-f46.google.com with SMTP id u5so32338478ejn.8 for ; Wed, 31 Mar 2021 15:07:00 -0700 (PDT) X-Gm-Message-State: AOAM533LLrCnkkwlla+OJKrvBg4ToBOa/ENnXhOSZf+UDTlhyryy3EA+ rdKPLJqjYaaDB1Xhg3TMjbnoR10atI/iwwBOVA== X-Google-Smtp-Source: ABdhPJwrN3REAzz1OFnLicmVPUk5n1+yxRC0vvnXdJFX4tBIzONHIoaDA5yrbSrYr/JFf8dREgK9I8MOVvtnzjaBbz4= X-Received: by 2002:a17:906:2312:: with SMTP id l18mr6123769eja.468.1617228418909; Wed, 31 Mar 2021 15:06:58 -0700 (PDT) MIME-Version: 1.0 References: <20210311000837.3630499-1-robh@kernel.org> <20210311000837.3630499-5-robh@kernel.org> In-Reply-To: From: Rob Herring Date: Wed, 31 Mar 2021 17:06:46 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v6 04/10] libperf: Add evsel mmap support To: Jiri Olsa Cc: Will Deacon , Catalin Marinas , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Ian Rogers , Alexander Shishkin , Honnappa Nagarahalli , Zachary.Leaf@arm.com, Raphael Gault , Jonathan Cameron , Namhyung Kim , Itaru Kitayama , linux-arm-kernel , "linux-kernel@vger.kernel.org" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210331_230704_915693_7BDD1427 X-CRM114-Status: GOOD ( 23.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Mar 12, 2021 at 12:29 PM Jiri Olsa wrote: > > On Fri, Mar 12, 2021 at 07:34:39AM -0700, Rob Herring wrote: > > On Fri, Mar 12, 2021 at 6:59 AM Jiri Olsa wrote: > > > > > > On Wed, Mar 10, 2021 at 05:08:31PM -0700, Rob Herring wrote: > > > > > > SNIP > > > > > > > + > > > > static int > > > > sys_perf_event_open(struct perf_event_attr *attr, > > > > pid_t pid, int cpu, int group_fd, > > > > @@ -137,6 +147,8 @@ void perf_evsel__free_fd(struct perf_evsel *evsel) > > > > { > > > > xyarray__delete(evsel->fd); > > > > evsel->fd = NULL; > > > > + xyarray__delete(evsel->mmap); > > > > + evsel->mmap = NULL; > > > > } > > > > > > > > void perf_evsel__close(struct perf_evsel *evsel) > > > > @@ -156,6 +168,45 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu) > > > > perf_evsel__close_fd_cpu(evsel, cpu); > > > > } > > > > > > > > +int perf_evsel__mmap(struct perf_evsel *evsel, int pages) > > > > +{ > > > > + int ret, cpu, thread; > > > > + struct perf_mmap_param mp = { > > > > + .prot = PROT_READ | PROT_WRITE, > > > > + .mask = (pages * page_size) - 1, > > > > + }; > > > > > > I don't mind using evsel->fd for dimensions below, > > > but we need to check in here that it's defined, > > > that perf_evsel__open was called > > > > Right, so I'll add this here: > > > > if (evsel->fd == NULL) > > return -EINVAL; Actually, pretty much none of the API checks this. perf_evsel__run_ioctl(), perf_evsel__enable(), perf_evsel__disable(), perf_evsel__read() will all just deference evsel->fd. So fix all of these or follow existing behavior? > > Note that struct evsel has dimensions in it, but they are only set in > > the evlist code. I couldn't tell if that was by design or mistake. > > we do? we have the cpus and threads in perf_evsel no? Right, perf_evsel has cpus and threads pointers which in turn have the sizes, but those pointers are not set within the evsel code. > also you'd need perf_evsel not evsel, right? > > > > > BTW, I just noticed perf_evsel__open is leaking memory on most of its > > error paths. > > nice.. let's fix it ;-) NM, I missed that they are static... Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel