From nobody Fri Apr 28 20:29:07 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Q7PMz0xHFz47wJ0; Fri, 28 Apr 2023 20:29:19 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Q7PMz0GNDz3Ljl; Fri, 28 Apr 2023 20:29:19 +0000 (UTC) (envelope-from kevans@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682713759; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+urTx6100qY7ME2NcvNjTYfBlftMxgEasnJN5/1MEYk=; b=y2fLPjHHGPOrxif4pwp/9NSkTS3wQazYGPgAZW9nVBvfQYXSCyVC4a3t9RElwSbz8hbiCv JddEDoCS2vLp2555ZU6yhoMc5EDB+ZejAvAnQuZV9lpv+Q+EJan//0AfquMiKqbJflmodE zaebF0q/qVFW9BtVPPmUPZraqqANEQFIhCxlBh8HPmfDDI1rsF6W1aoP9HneRyDCa9zBJT uLWdgQPaapI2w7bItwsmN3T3PJydy5TqpwALmKFucMfg6TPQ6Yu3APm8Qro2J0bk8FRI80 vzoLrGri2DdRLZi7kyPopBJB5P/rByLYQtrdllnOpcJ4qfnY4bVhBBpkJPi6SA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682713759; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+urTx6100qY7ME2NcvNjTYfBlftMxgEasnJN5/1MEYk=; b=Q1Peba2W6iZ9okdk01qsazPH1svf5CIiht5iSrPPwbTJB9ccdJGdP3+sEucsx4ynR8QQCz 7crni4tw/TpcvPAUq4NULJrIgcC2fOxeQjd0cPmbrGp89p28I9ztAx4uHuA/5Xnp1w9yUe RoYMGi1hzLUSPui6yL6h8haKQ1C5bXtfbUFpzkBivhIsSO75JncXk2wu6vGDslf0U2tdol wTjDDPXnAVdUyvLwQOvcwXr5atM/xZyZxkDB9vWJxGJB6YXENOD7EJa/0i/r9tdSlAAYjc AbJ4npdWKfArfIMzzRmNWsH16O6W4KBvwcXZoLl2NL0fvf+21EWtqSwZkZ6KSg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1682713759; a=rsa-sha256; cv=none; b=f2AlxLccRDG1sgbfy/6o1h3E7XmjvVQz3UgfzfDhaDw5rf9mLqDXGx+thXN8P0qWk4UBC3 09z111KesM6WlGZ4/M7OK0fCWGjtqPU3klzWOAp/io8dSq3NXLKSlV0Mqor/Ht6d8UxfnH xgoxCEKw52qTdiX5Tsf4HKig/if7yvcofNuCXHkW0KSCfjlnl77oIbMcN8E+1XT0cMm5Vk XbEXJzVGHTCjsVW9MEhMWkNsGdkgjt+IbnjE5SmrPrHmptDncArdimq9gePKDlI/llfgY2 AqBM+9BZc5eoinnTjKZONedqCdHtScmZlXoU/Hyj8TC9AlGU6tXrNy3eYbIT1Q== Received: from mail-qv1-f49.google.com (mail-qv1-f49.google.com [209.85.219.49]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) (Authenticated sender: kevans) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Q7PMy6J1WzSCw; Fri, 28 Apr 2023 20:29:18 +0000 (UTC) (envelope-from kevans@freebsd.org) Received: by mail-qv1-f49.google.com with SMTP id 6a1803df08f44-5ef41569a3fso2563136d6.3; Fri, 28 Apr 2023 13:29:18 -0700 (PDT) X-Gm-Message-State: AC+VfDzsNAOg3cGB/5FMk+QaHDDtB1vpaqQiuxcwntIU9vhgvaejjALY d3Ikfm8GM9ouGNNNy5ElHGUy4j9x/NBbk/8EzDs= X-Google-Smtp-Source: ACHHUZ4TkkxZKlzTvv5aK8RFIKR+Hh3T5wR6BuxxoXKj9ehmlnR8dPa22fARFFS0RlYMAfFOBOKtT3/l8ShJef8mCP0= X-Received: by 2002:ad4:5e8e:0:b0:5cc:e059:efa3 with SMTP id jl14-20020ad45e8e000000b005cce059efa3mr12109996qvb.23.1682713758423; Fri, 28 Apr 2023 13:29:18 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202205310005.24V05MTi034088@gitrepo.freebsd.org> <5593fcdc-9adb-89a9-3ad8-151a9e7c5c0d@FreeBSD.org> In-Reply-To: <5593fcdc-9adb-89a9-3ad8-151a9e7c5c0d@FreeBSD.org> From: Kyle Evans Date: Fri, 28 Apr 2023 15:29:07 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: git: c1e813d12309 - main - hwpmc: Correct selection of Intel fixed counters. To: Alexander Motin Cc: Kyle Evans , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, "Sideropoulos, Alexander" , Andrew Gallatin , Toomas Soome Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-ThisMailContainsUnwantedMimeParts: N On Thu, Apr 20, 2023 at 1:57=E2=80=AFPM Alexander Motin w= rote: > > Kyle, Andrew, > > On 19.04.2023 16:06, Kyle Evans wrote: > > On Mon, May 30, 2022 at 7:05=E2=80=AFPM Alexander Motin wrote: > >> > >> The branch main has been updated by mav: > >> > >> URL: https://cgit.FreeBSD.org/src/commit/?id=3Dc1e813d1230915e19a236ec= 687cadc1051841e56 > >> > >> commit c1e813d1230915e19a236ec687cadc1051841e56 > >> Author: Alexander Motin > >> AuthorDate: 2022-05-30 23:46:48 +0000 > >> Commit: Alexander Motin > >> CommitDate: 2022-05-31 00:05:15 +0000 > >> > >> hwpmc: Correct selection of Intel fixed counters. > >> > >> Intel json's use event=3D0 to specify fixed counter number via um= ask. > >> Alternatively fixed counters have equivalent programmable event/u= mask. > >> > >> MFC after: 1 month > >> --- > >> sys/dev/hwpmc/hwpmc_core.c | 34 +++++++++++++++++++++++++--------- > >> 1 file changed, 25 insertions(+), 9 deletions(-) > >> > >> diff --git a/sys/dev/hwpmc/hwpmc_core.c b/sys/dev/hwpmc/hwpmc_core.c > >> index fce97cbd5b8e..73cfee0b3975 100644 > >> --- a/sys/dev/hwpmc/hwpmc_core.c > >> +++ b/sys/dev/hwpmc/hwpmc_core.c > >> @@ -245,15 +245,31 @@ iaf_allocate_pmc(int cpu, int ri, struct pmc *pm= , > >> ev =3D IAP_EVSEL_GET(config); > >> umask =3D IAP_UMASK_GET(config); > >> > >> - /* INST_RETIRED.ANY */ > >> - if (ev =3D=3D 0xC0 && ri !=3D 0) > >> - return (EINVAL); > >> - /* CPU_CLK_UNHALTED.THREAD */ > >> - if (ev =3D=3D 0x3C && ri !=3D 1) > >> - return (EINVAL); > >> - /* CPU_CLK_UNHALTED.REF */ > >> - if (ev =3D=3D 0x0 && umask =3D=3D 0x3 && ri !=3D 2) > >> - return (EINVAL); > >> + if (ev =3D=3D 0x0) { > >> + if (umask !=3D ri + 1) > >> + return (EINVAL); > > > > Hey Alexander, > > > > This seems to break system-mode sampling of fixed-mode counters; e.g., > > from the manpage, `pmcstat -S instructions`, and I'm not at all > > familiar with these parts of pmc. We come in once with umask=3D1, ri=3D= 0; > > then again with umask=3D1, ri=3D1. The latter fails and we try umask=3D= 1, > > ri=3D2, which again fails, and the PMCALLOCATE op ultimately fails. > > > > Is `umask !=3D ri + 1` correct, or should this be reverted back to > > `umask =3D=3D 0x3 && ri !=3D 2` or something else? I've no idea what th= e > > `umask` values represent in this context. > > I still believe this code is correct, at least as much as data in > lib/libpmc/pmu-events/arch/x86 is consistent (look for > "INST_RETIRED.ANY", that actually fails here after translation from > "instructions"). The multiple calls you see is the result of hwpmc > trying to find counter "matching" parameters. For fixed counters the > match is really just (umask =3D=3D ri + 1), since umask for fixed counter= s > in the Intel events data is just a counter number starting from 1. > > I've tried to test fixed-mode counters myself and got it failing also. > But looking a bit deeper, I think the problem may be caused by this > patch: https://reviews.freebsd.org/D35709 . I think it tries to > allocate the same fixed counter twice, where the first attempt succeeds, > while the second fails, since the hardware is already busy. I haven't > looked close on the pmcstat code, but I suppose the patch was wrong, so > if you or Andrew wish to fix it properly -- I'd be happy. > Hi, I looked at the initial review where Andrew reported his issue: https://reviews.freebsd.org/D35342 -- I think we need to take a closer look at understanding how the original issue happened. I note: Core was generated by `pmcstat -R out.pmclog_cpi01 -z 32 -G out.pmcstat_cpi= 01'. With those flags, the patch in D35709 could not have actually fixed the assertion. Note that the -R flag sets FLAG_READ_LOGFILE: https://cgit.freebsd.org/src/tree/usr.sbin/pmcstat/pmcstat.c#n825 There's a sanity check a little later; if any `args.pa_events` are specified (-p/-s/-P/-S), it throws an error: https://cgit.freebsd.org/src/tree/usr.sbin/pmcstat/pmcstat.c#n950 Thus, we have nothing to iterate over, and the pmc_allocate() call at https://cgit.freebsd.org/src/tree/usr.sbin/pmcstat/pmcstat.c#n1188 is not actually reachable in any circumstance with -R specified. I think 0aa150775179a4f683f should be backed out, but it's also important to figure out what actually happened to induce those crashes and why it has apparently disappeared in the interim. Thanks, Kyle Evans