From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="eryl6Wgv" Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A93EFB9 for ; Thu, 23 Nov 2023 10:00:12 -0800 (PST) Received: by mail-ed1-x52c.google.com with SMTP id 4fb4d7f45d1cf-548db776f6cso13030a12.1 for ; Thu, 23 Nov 2023 10:00:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1700762411; x=1701367211; darn=vger.kernel.org; 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=oC1A8FYWCE9VFXIWJCcHsK/1/70L94HS4n6KVdO2o8A=; b=eryl6WgvluWuvqsuvNz+n6Cin8fCM1fVm+YqTcpyHHvgvM1HkxyvVPtynz5R9YOEtw lalRDk7PV2k2PUKSYNIT4MZjx2NjV7vPwnaPaufc7h/XqXqz8/jAMi+ZZMmYUst8B2eV bWje0hX1zICLBmQol88EnygPKeBOsXYfhEDCwNsY5goWKcreRwutla00Lg7VzBD76HF7 gaMzI+RNl0fpDio8LxQSHBLGp3rleQp/JKjWcogK28xrJQtJbKDQdJOjOUP+KUDXiKIM ch5dSMWzhSQAfMGat9YSJ1CUqqg37N+YuT4+NCF0sj6PyYtLusxpataKI1d/tyWQrEGG lwJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700762411; x=1701367211; 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=oC1A8FYWCE9VFXIWJCcHsK/1/70L94HS4n6KVdO2o8A=; b=jj62kcBjpkZZeaWdmaz9sCWpk5ZdFekn/kY9ybhM0I++QcY6enBYfgR/lZ/B6Iq++2 gjL42+I5Xxv1EsQj2Ol8J8ZOE/ezRUCeUYbEfFdQoyyxBeo6eWXAqf2qMYcuaMT7MHzQ 0nuknEuC0KhyKly8tgXWgLSdXnyKtsrVVarkyh2X1mUB7IZHm9wBPwh+nCcGUcyAQV3o ZwjUaBShnTSBhn0TV/zZ/zT86eF0JUlq+ZEuEYtBQ5HololG0ubpk7KcLi8kXhbMsYwo EMksAG6MFJDtrSSbDXDLzzGysy2l2cwha6O/1oW9TTVckUkiIyLebVKosB1KOjrVuV0t 40hg== X-Gm-Message-State: AOJu0YwmToeWNNX1f8pl43UVMwPgCxG08jBj+Oy/l44aGFQcCYt9zqua pS8nKHWJlERepElHNyF0BMhee3pUeYZVSF4EzRJF5A== X-Google-Smtp-Source: AGHT+IHoBIIXEGSlxDzz0Dj3khuJipljirn6ek0M2FA+OCRKBDUg44tbPGTWEee8EfzTFB6XoqrHGN7ahfjVHOEnbQM= X-Received: by 2002:a05:6402:540a:b0:545:279:d075 with SMTP id ev10-20020a056402540a00b005450279d075mr327312edb.1.1700762410841; Thu, 23 Nov 2023 10:00:10 -0800 (PST) Precedence: bulk X-Mailing-List: linux-perf-users@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20231123042922.834425-1-irogers@google.com> <86cyw0zeiu.wl-maz@kernel.org> <86bkbkzc2p.wl-maz@kernel.org> In-Reply-To: <86bkbkzc2p.wl-maz@kernel.org> From: Ian Rogers Date: Thu, 23 Nov 2023 09:59:59 -0800 Message-ID: Subject: Re: [RFC PATCH v1] perf parse-events: Make legacy events lower priority than sysfs/json To: Marc Zyngier Cc: Mark Rutland , Hector Martin , Arnaldo Carvalho de Melo , Namhyung Kim , Peter Zijlstra , Ingo Molnar , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Kan Liang , 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 On Thu, Nov 23, 2023 at 8:09=E2=80=AFAM Marc Zyngier wrote= : > > On Thu, 23 Nov 2023 15:27:54 +0000, > Ian Rogers wrote: > > > > On Thu, Nov 23, 2023 at 7:16=E2=80=AFAM Marc Zyngier w= rote: > > > > > > Again, perf gets shipped in distros, and not necessary as the latest > > > version. Rather, they tend to ship the version matching the kernel. N= o > > > backport, buggy perf. > > > > Please complain to the distros. I complained to Debian, we got rid of > > the horrible wrapper script thing they did. I complained to two > > separate Ubuntu people over the last two weeks as they still have > > broken packaging even though they derive from Debian. Fedora is of > > course perfect as Arnaldo oversees it :-) > > In this instance, I don't need to complain to anyone but you. And > guess what: it is on Fedora that this issue was first discovered. > > I also don't see what distro packaging policy has anything to do with > the issue at hand, but that's beside the point. Because the latest perf tool is always improved and carries fixes, just as say gcc or clang. We don't ask these tools to backport fixes and then deliberately run out-of-date versions of them. > > > > > And again, I don't see a bug in the PMU driver. > > > > Whether the PMU driver is requested a legacy cycles event or the > > cycles event as an event code, the PMU driver should support it. > > Supporting legacy events is just something core PMU drivers do. This > > workaround wouldn't be necessary were it not for this PMU bug. > > Again, *which* PMU bug? What is a legacy event, and when has this > terminology made it into the kernel? Who has decided that a change was > necessary? Why haven't you submitted patches upgrading all the PMU > drivers to support whatever you are referring to? I did fix ARM's PMU driver for extended types, James Clark took over the patch. The term legacy has at least been in use in kernel source code for over 11 years: http://lkml.kernel.org/r/1337584373-2741-4-git-send-email-jolsa@redhat.com An issue I face in fixing somebody's PMU driver is it is ever so useful to be able to test. The work done with James was done blind by me except for checking for regressions on a raspberry pi 4, which isn't heterogeneous (nor is the 5 *sigh*). The fact there were bugs in ARM's PMU driver for so long shows a lack of testing by ARM and we've been going out of our way to increase testing. Something positive ARM could do in this area is to update the parse-events test, yes the one that is supposed to test issues like this, so that the hardcoded "cpu" PMU assumption that works on most platforms who name their core PMU "cpu" also works on ARM. For bonus points setting up testing so that we know when things break would be useful. As mentioned in previous emails I hope to work away from needing an actual machine to test the perf tool's correctness, but we're a long way from that. There are very many BIG.little Android devices in the field where the PMUs are not set up as heterogeneous, ARM could contribute a CTS test to Android to make sure this doesn't happen. Thanks, Ian > > This change impacts every user of perf not just a partial fix to > > workaround ARM PMU driver issues, see the updated parse-events test > > for a list of what a simple test sees as a behavior change. > > When making far-reaching changes to a subsystem, I apply two rules: > > - I address everything that is affected, not just my pet architecture > > - I don't break other people's toys, which means compatibility is a > *must*, not a 'nice to have' > > By this standard, your complaining that "ARM is broken" doesn't hold. > It was working just fine until your changes rendered perf unusable. > > Nonetheless, thank you for addressing it quickly. This is sincerely > appreciated. > > M. > > -- > Without deviation from the norm, progress is not possible.