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 654A43F4127; Thu, 14 May 2026 04:35:18 +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=1778733318; cv=none; b=iRh2rqPww6djEPm+KHFUMjklkAn9+JR71EDjzojE0cJS757vRVsau3646Ngi3fLUsZGVsHR6W7n6lpBwciAf2bFoIPhN8Xq3cxSq6hSetk9TDAm/L0sdQC66CToFbAfL1sVqAGJ0AfC+IqqvKS9NW+XVpeiFIHPwTrfYeQAt8nI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778733318; c=relaxed/simple; bh=1ssLU2f3czkBnxWmcWahoHkeXwL7OgSqF0DYp06kaEU=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=VX9H+gcKnmHa8AwbbkZec4P7Nnuf9Wh5z5lMmEG0GVLneZxeNBdDUCSrJBEZCZq+lFPLtw9NBpBlDd4XWYdEMUAMbqryLZEhBM+Ss3eVa/Qyd/DEtLQ+goXpIsn9lHhIxX3KJmjib7EVLw7lZxFlwPuMyXJwCFZZtyalJmZvhp8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=EqB1ukED; 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="EqB1ukED" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2016AC2BCB7; Thu, 14 May 2026 04:35:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778733318; bh=1ssLU2f3czkBnxWmcWahoHkeXwL7OgSqF0DYp06kaEU=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From; b=EqB1ukEDfPbR0fJD4tUyyoTgta8WCeK8GJl13RWOvJ13q0qA9WSuIZeAgTkZhtind AwaMkGjNhXI6IEYvbJ9Db8omzpajGaaJ+1akqF4LPLGhO+a1hz1MI6OcPlpbODA3UE RCFVU2JNsY4lGGp6UpYukNN9Q43Uj1hP+IOugPRz96BUV4q6Zx7VU8s2QpkfMxgrng yof735x78Ra/900GUpp9MU/jnYa+lzXF7HozRQ2Ux1xnhK/rnf38KZrBpg05DM41ur fqDAUilzQDMmJ4pFvO58OPHuFxdcnJpo1VNCF1Oea2ah5bDvrzdC1vpcVBlVXvM8H3 FCYzVVybgWXNA== From: sashiko-bot@kernel.org Subject: Re: [PATCH v2 14/18] perf pmu-events: Split big_c_string storage into standalone compilation unit Reply-To: sashiko-reviews@lists.linux.dev To: "Ian Rogers" Cc: bpf@vger.kernel.org, linux-perf-users@vger.kernel.org In-Reply-To: <20260512174638.120445-15-irogers@google.com> References: <20260512174638.120445-15-irogers@google.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Thu, 14 May 2026 04:35:17 +0000 Message-Id: <20260514043518.2016AC2BCB7@smtp.kernel.org> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [High] The use of the `&:` grouped target syntax in the Makefile breaks c= ompatibility with GNU Make < 4.3 and causes race conditions during parallel= builds. - [Low] The newly generated `pmu-events-string.c` is not added to `tools/pe= rf/.gitignore`, resulting in an untracked file after an in-tree build. -- commit e5e31a4f692737db305f31f8a7cee6c36037c141 Author: Ian Rogers perf pmu-events: Split big_c_string storage into standalone compilation unit Currently, jevents.py emits both the massive 2.8 MB big_c_string literal and tens of thousands of compact_pmu_event struct arrays into a single pmu-events.c compilation unit. This commit refactors jevents.py to emit big_c_string into a dedicated pmu-events-string.c compilation unit. This allows Kbuild to compile them simultaneously in parallel, cutting C compilation latency. > diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build > index dc1df2d57ddc1..e957aa2fcb480 100644 > --- a/tools/perf/pmu-events/Build > +++ b/tools/perf/pmu-events/Build > @@ -1,7 +1,12 @@ > EMPTY_PMU_EVENTS_C =3D pmu-events/empty-pmu-events.c > # pmu-events.c will be generated by jevents.py or copied from EMPTY_PMU_= EVENTS_C > PMU_EVENTS_C =3D $(OUTPUT)pmu-events/pmu-events.c > +PMU_EVENTS_STRING_C =3D $(OUTPUT)pmu-events/pmu-events-string.c > + Does this new generated file need to be added to tools/perf/.gitignore? Looking at tools/perf/.gitignore, pmu-events/pmu-events.c is explicitly ignored, but the new string file is not: tools/perf/.gitignore: ... pmu-events/pmu-events.c pmu-events/jevents pmu-events/metric_test.log ... Compiling perf in-tree might leave the working directory dirty with an untracked pmu-events-string.c file. > pmu-events-y +=3D pmu-events.o > +ifneq ($(NO_JEVENTS),1) > +pmu-events-y +=3D pmu-events-string.o > +endif > =20 > # pmu-events.c file is generated in the OUTPUT directory so it needs a > # separate rule to depend on it properly [ ... ] > @@ -222,8 +232,9 @@ endif > =20 > # Finally, the rule to build pmu-events.c using jevents.py. All test > # and inputs are dependencies. > -$(PMU_EVENTS_C): $(JEVENTS_DEPS) > +$(PMU_EVENTS_STRING_C) $(PMU_EVENTS_C) &: $(JEVENTS_DEPS) > $(call rule_mkdir) Will this rule work correctly on systems using GNU Make older than 4.3? The top-level Makefile enforces a minimum requirement of GNU Make 4.0, but the &: grouped target feature was introduced in GNU Make 4.3. On older, supported versions of Make (like 4.2.1), the &: syntax is not recognized. Make parses & as a literal target and interprets the line as having three independent targets: $(PMU_EVENTS_STRING_C), $(PMU_EVENTS_C), and &. During a parallel build, could this result in multiple jevents.py processes spawning simultaneously? It seems they might write to pmu-events.c and pmu-events-string.c concurrently, potentially interleaving output and corrupting the generated C source files. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260512174638.1204= 45-1-irogers@google.com?part=3D14