From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 639836AF8A; Thu, 22 Feb 2024 19:54:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708631693; cv=none; b=CiwyNpVuCRhRudB7sRpxbVJ7df6Juz7CUDDABUO2jqTJSQVIhvyiZERBq/kSz6lOpLFwIWgnU0mg7GW4JSCVuYkZPk/z5Ez7kAS8DXNHKjbYjY+G5lSHFerDuTPacsK1zU3vYK+nEBqDEKQZ3GMUuI/l7B50rTsUVWXGyEVTQnY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708631693; c=relaxed/simple; bh=bSjeGRhKKcsTliuguvmBrYAYCXALQGnt4jTIGPUKbxI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=s+wQpKSs+ZkmfOxQbONs3A3gxOr+90b+11iYXxgmMjV3d6ckfL4t2kslZzX+nvXyHjqD9EWTYClFr3KqPduGtaTcxxeM1RrGH3w/pD78ym/oG39meniI+boHoGn+7ReCFanUlxQ62rmhOxkzZKvLG0eW95MNjS8TH0qxMObllDU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=d01xQM2W; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="d01xQM2W" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 89FEFC433F1; Thu, 22 Feb 2024 19:54:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1708631692; bh=bSjeGRhKKcsTliuguvmBrYAYCXALQGnt4jTIGPUKbxI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=d01xQM2WWi8U9v1enS4CDg7faOEehZVjZL+xrVT3C73d0cyZMAy0kMaClxhHeD1Eg RUAcBZdhrOz1FprZsrSo/rdCS5FRFCiLHgp3xLfSzlexVbLyIibEZR2zrAcONB9Yoj 4Xdm77/5d43k/FgM4YoDgGvlzsEBtO/rzV+t5P2zxbvQyISASUOhcSFsp/jaqEMhHD DJodJH7ZlpB/LJZEVpny04B439ZJl79A19pdu2dK+eUJnZ45YJEpr1fIBL0+LrS0nT FBctZo46GCxwciW7vkKhDeKILNovNB3gd9rCCesYVbT1+arnSr8qUjSlDyHOYtGlzo pfrhZdfpn8jKQ== Date: Thu, 22 Feb 2024 16:54:49 -0300 From: Arnaldo Carvalho de Melo To: Kyle Huey Cc: Ian Rogers , Kyle Huey , Robert O'Callahan , Peter Zijlstra , Ingo Molnar , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] perf test: Test FASYNC with watermark wakeups. Message-ID: References: <20240221175210.19936-1-khuey@kylehuey.com> <20240221175210.19936-2-khuey@kylehuey.com> Precedence: bulk X-Mailing-List: linux-perf-users@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu, Feb 22, 2024 at 09:55:36AM -0800, Kyle Huey wrote: > On Wed, Feb 21, 2024 at 10:36 AM Ian Rogers wrote: > > On Wed, Feb 21, 2024 at 9:52 AM Kyle Huey wrote: > > > + if (fcntl(fd, F_SETFL, FASYNC)) { > > > + pr_debug("failed F_SETFL FASYNC %d\n", errno); > > > + goto cleanup; > > > + } > > Thanks for the work! The perf tool and perf test should run on older > > kernels ideally without failure. I'm assuming this would fail on an > > older kernel. Could we make the return value skip in that case? > Ah, hmm, I wasn't aware of that. This would fail on an older kernel, > yes. It's not possible to distinguish between an older kernel and a > kernel where this fix broke (at least not without hardcoding in an > expected good kernel version, which seems undesirable), so that would Take a look at: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/tools/perf/tests/sigtrap.c?id=650e0bde43f35bb675e87e30f679a57cfa22e0e5 To see how introspecting using BTF can be used to figure out if internal data structures have what is needed, or if functions with some specific arguments are present, etc, for sigtrap we have, in the patch above: - TEST_ASSERT_EQUAL("unexpected sigtraps", ctx.signal_count, NUM_THREADS * ctx.iterate_on); + expected_sigtraps = NUM_THREADS * ctx.iterate_on; + + if (ctx.signal_count < expected_sigtraps && kernel_with_sleepable_spinlocks()) { + pr_debug("Expected %d sigtraps, got %d, running on a kernel with sleepable spinlocks.\n", + expected_sigtraps, ctx.signal_count); + pr_debug("See https://lore.kernel.org/all/e368f2c848d77fbc8d259f44e2055fe469c219cf.camel@gmx.de/\n"); + return TEST_SKIP; + } else + TEST_ASSERT_EQUAL("unexpected sigtraps", ctx.signal_count, expected_sigtraps); And then: +static bool kernel_with_sleepable_spinlocks(void) +{ + const struct btf_member *member; + const struct btf_type *type; + const char *type_name; + int id; + + if (!btf__available()) + return false; + + id = btf__find_by_name_kind(btf, "spinlock", BTF_KIND_STRUCT); + if (id < 0) + return false; + + // Only RT has a "lock" member for "struct spinlock" + member = __btf_type__find_member_by_name(id, "lock"); + if (member == NULL) + return false; + + // But check its type as well + type = btf__type_by_id(btf, member->type); + if (!type || !btf_is_struct(type)) + return false; + + type_name = btf__name_by_offset(btf, type->name_off); + return type_name && !strcmp(type_name, "rt_mutex_base"); +} > mean the test would always return ok or skip, not ok or fail. Is that > ok? It should return: Ok if the kernel has what is needed and the test passes Skip if the test fails and the kernel doesn't have what is needed FAIL if the test fails and the kernel HAS what is needed. 'perf test sigtrap' also checks for the presence of the feature it requires: static bool attr_has_sigtrap(void) { int id; if (!btf__available()) { /* should be an old kernel */ return false; } id = btf__find_by_name_kind(btf, "perf_event_attr", BTF_KIND_STRUCT); if (id < 0) return false; return __btf_type__find_member_by_name(id, "sigtrap") != NULL; } fd = sys_perf_event_open(&attr, 0, -1, -1, perf_event_open_cloexec_flag()); if (fd < 0) { if (attr_has_sigtrap()) { pr_debug("FAILED sys_perf_event_open(): %s\n", str_error_r(errno, sbuf, sizeof(sbuf))); } else { pr_debug("perf_event_attr doesn't have sigtrap\n"); ret = TEST_SKIP; } goto out_restore_sigaction; } - Arnaldo