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=-7.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no 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 87287C433E2 for ; Fri, 4 Sep 2020 05:53:01 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 33AA3206C9 for ; Fri, 4 Sep 2020 05:53:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="0ShfbJ5e"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="bfchoPp5" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 33AA3206C9 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com 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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id: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=tt4GXumrFqxZl6qNLuy8ua/CV2uXiT2Do965pv6vE38=; b=0ShfbJ5eFpMtzhZKKBeF/JYYX krSbKvkkd5cN4tDPqaqhFt3pk93MxSh2CmhlV6BW11f09qpkpKTW2ymzy5t66uKC8YnBIxEZTApjw wSQSVbGuYF7anSgH02NSsqJrr6774ulI5L0wTWy88W+W0n2YrZCEMvoCgtVcBwmjhmWAr8m2A2UHV gLWg9qvgPDY0fNezCkOiFQi6743AQPf7zWrAV7jRm/EXLApdRA7SYWSJxLQ2m8yC/7WrSBf9wHhwF 3NXMJ/UQ9qV/0yEssVuKXO/69cHGLb1Y7wVvU/LqcoBahx6bsLrLrWlqBCiSXX0JHdl9Ec5ouiIjc /lOgpFnWA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kE4dA-0004ul-65; Fri, 04 Sep 2020 05:51:24 +0000 Received: from mail-wm1-x342.google.com ([2a00:1450:4864:20::342]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kE4d7-0004u2-4x for linux-arm-kernel@lists.infradead.org; Fri, 04 Sep 2020 05:51:22 +0000 Received: by mail-wm1-x342.google.com with SMTP id a65so4888431wme.5 for ; Thu, 03 Sep 2020 22:51:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VBkALWNFmSZj7rK+SmejfUE4FzmI9+bkW+wvdb/tgIU=; b=bfchoPp5zj2lItpqUucfpnTEhQD1FHHPAGOIkeuUZVWDDo5HxG8ybEi/qEjt2BAbRz gPGLG3VT9Y4mAmhzVVhYijXN7UOUvEbSB58Fvx0Pm4TidSPoINzyIYjEdldmE2y8zMZv QZsUB1GVUy4/D2xHNbaOKupKDg82JDojrtPLY1/bEzlVsQYFPQwf4Ok2+wbWzj+yyfah h8m8EbNu+OK1CqM5QLswpmmBD9Nhue8UnY71/09ccFOYwvpsi8KYfb28cATjtBoaqdgn tWV0Tsqjs1KiKpVPen4DB4C8hThzO+BYqj24APl2mMnfW7mEggKTMs3HpgEqFNkfIQzN MsXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VBkALWNFmSZj7rK+SmejfUE4FzmI9+bkW+wvdb/tgIU=; b=HtKqwZaSZn2BwIWGz5QYjM2FFCpMIQvzY/y7YLJ8lUf6VJ/5d3jwR2CUCzPSEVERH3 n65uhqXUW1eo/vnv/Nx5uMqmYCDkzsdDRo5B1ef3chC+MmXpXPdfeyU0MaDPc7WhBH/H HZHLCyx8hTLSat8adg4pRbIDy/ybAsLl/OqYKXwDGeRunh1ztC+rIfWjrQE56Woh6Do3 34rjcti8XSiNGOD1DZdvWQixwBGcTHV+TKX6oeQo7Zp71HBW2oS9Ulrlxlg2eSHyyWmx CR2sef1oc0Urr1rWiMTqrJ8sLtnt+h7l3bBgE8gGVR39iBznhVbUY4zduwcYU8Y/R6Qa I+nA== X-Gm-Message-State: AOAM533UHvF/fBFSCzqN5ob2knNWstumC7/e8VYCmSdCSBrvGaXtW84S 97bJKVOahPsXQLrYut+3wRpeHenX4+PljpET8xcyAw== X-Google-Smtp-Source: ABdhPJyjfg4RTVOS1EmVbuIhdOMU+V8UOMkY75qdg9ilr5NV+1cnP9l4YOD2cVaYmZAtsmnQPCN//Uc++UbX7cPSGkU= X-Received: by 2002:a05:600c:22d1:: with SMTP id 17mr1830382wmg.58.1599198679685; Thu, 03 Sep 2020 22:51:19 -0700 (PDT) MIME-Version: 1.0 References: <20200828205614.3391252-1-robh@kernel.org> <20200828205614.3391252-6-robh@kernel.org> In-Reply-To: From: Ian Rogers Date: Thu, 3 Sep 2020 22:51:08 -0700 Message-ID: Subject: Re: [PATCH v2 5/9] libperf: Add support for user space counter access To: Rob Herring X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200904_015121_256769_1E48CAC9 X-CRM114-Status: GOOD ( 37.11 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Will Deacon , Peter Zijlstra , Catalin Marinas , LKML , Arnaldo Carvalho de Melo , Alexander Shishkin , Raphael Gault , Ingo Molnar , Honnappa Nagarahalli , Jonathan Cameron , Namhyung Kim , Jiri Olsa , Linux ARM 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 Wed, Sep 2, 2020 at 12:48 PM Rob Herring wrote: > > On Wed, Sep 2, 2020 at 12:07 PM Ian Rogers wrote: > > > > On Fri, Aug 28, 2020 at 1:56 PM Rob Herring wrote: > > > > > > x86 and arm64 can both support direct access of event counters in > > > userspace. The access sequence is less than trivial and currently exists > > > in perf test code (tools/perf/arch/x86/tests/rdpmc.c) with copies in > > > projects such as PAPI and libpfm4. > > > > > > In order to support usersapce access, an event must be mmapped. While > > > there's already mmap support for evlist, the usecase is a bit different > > > than the self monitoring with userspace access. So let's add a new > > > perf_evsel__mmap() function to mmap an evsel. This allows implementing > > > userspace access as a fastpath for perf_evsel__read(). > > > > > > The mmapped address is returned by perf_evsel__mmap() primarily for > > > users/tests to check if userspace access is enabled. > > > > > > Signed-off-by: Rob Herring > > > --- > > > > +int perf_mmap__read_self(struct perf_mmap *map, struct perf_counts_values *count) > > > +{ > > > + struct perf_event_mmap_page *pc = map->base; > > > + u32 seq, idx, time_mult = 0, time_shift = 0; > > > + u64 cnt, cyc = 0, time_offset = 0, time_cycles = 0, time_mask = ~0ULL; > > > + > > > + BUG_ON(!pc); > > > + > > > + if (!pc->cap_user_rdpmc) > > > + return -1; > > > + > > > + do { > > > + seq = READ_ONCE(pc->lock); > > > + barrier(); > > > + > > > + count->ena = READ_ONCE(pc->time_enabled); > > > + count->run = READ_ONCE(pc->time_running); > > > + > > > + if (pc->cap_user_time && count->ena != count->run) { > > > + cyc = read_timestamp(); > > > + time_mult = READ_ONCE(pc->time_mult); > > > + time_shift = READ_ONCE(pc->time_shift); > > > + time_offset = READ_ONCE(pc->time_offset); > > > + > > > + if (pc->cap_user_time_short) { > > > + time_cycles = READ_ONCE(pc->time_cycles); > > > + time_mask = READ_ONCE(pc->time_mask); > > > + } > > > + } > > > + > > > + idx = READ_ONCE(pc->index); > > > + cnt = READ_ONCE(pc->offset); > > > + if (pc->cap_user_rdpmc && idx) { > > > + u64 evcnt = read_perf_counter(idx - 1); > > > + u16 width = READ_ONCE(pc->pmc_width); > > > + > > > + evcnt <<= 64 - width; > > > + evcnt >>= 64 - width; > > > + cnt += evcnt; > > > + } else > > > + return -1; > > > + > > > + barrier(); > > > + } while (READ_ONCE(pc->lock) != seq); > > > + > > > + if (count->ena != count->run) { > > > > There's an existing bug here that I tried to resolve in this patch: > > https://lore.kernel.org/lkml/CAP-5=fVRdqvswtyQMg5cB+ntTGda+SAYskjTQednEH-AeZo13g@mail.gmail.com/ > > Due to multiplexing, enabled may be > 0 but run == 0 and the divide > > below can end up with divide by zero. > > Yeah, I saw that, but didn't try to also fix that issue here. > > > I like the idea of this code being in a library, there's an intent > > that the perf_event.h and test code be copy-paste-able, but there is > > some pre-existing divergence. It would be nice if this code could be > > closer to the sample code in both the test and perf_event.h. > > The only way we get and keep all the versions of the code aligned is > removing the other copies. We should just remove the code comment from > perf_event.h IMO. If rdpmc.c is going to stick around given some > resistance to removing it, then perhaps it should be converted to use > libperf. At that point it could also be arch independent. Though I > don't like the idea of having the same test twice. This makes sense to me, perhaps others could comment. Given the cleaned up API fixing or deleting tools/perf/arch/x86/tests/rdpmc.c is desirable (as your patch set does). I wondered if we could do Jiri's suggestion to run the lib/perf tests with perf test. One way would be to have shell script wrapper in tools/perf/tests/shell. It's not clear how to make a dependency from a shell script there and tests built elsewhere in the tree though. > > As per the change above, I think running and enabled times need to be > > out arguments. > > They are now in this version. Sorry, my mistake. I'd missed that. Thanks, Ian > Rob _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel