Received: by 2002:a89:288:0:b0:1f7:eeee:6653 with SMTP id j8csp71531lqh; Mon, 6 May 2024 11:35:27 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCW2C/v1b7nLVyOtc3duyLWzgDAd3yYeIT5Hk7xAN2DLEqFWNH2sYSBPaPEU+KvlrvM1wWGZBkQTOWjDa7QH9Gokt+c48bJef8pzp1vz2Q== X-Google-Smtp-Source: AGHT+IGXFXKe3CnSe1CUJT80eP1IvYc9DuXsTNONrVvBtNNggK1qOoIdkZE3657XerD6vC2bMo/n X-Received: by 2002:a17:907:6026:b0:a59:9a68:7260 with SMTP id fs38-20020a170907602600b00a599a687260mr6412393ejc.62.1715020527080; Mon, 06 May 2024 11:35:27 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1715020527; cv=pass; d=google.com; s=arc-20160816; b=W/uNCDWJ0qa4ibzp2O+mQ+yaxGhss6zTqzf42KEfDenFCqMT0dmCySNbQhHUCxdfqi LyivpUYcpVwbbf/DUDKTeEw9D/PxrrhMlSJJ68++VNun9L5vcVZupxwFpY6PoefOajCF rJM9xsJJ6NZuDCbkKQ6kEvHCtCTWQwZGcNMQrWD1qGqwJrjnU39QjnTk+pklSsbB1Jbz z8Tg0GRYRP3GZMbi/qubT/UbExYU6X36eLWJ0/lA4UV8kDdQ9kJl4OYizuaHs+Knd4n9 k4q2wDAMjzfyh6Do10DN3mkVtouoYzjXI8MaH6uKq8OsAoHP84vpCUJ2kPXdn0e4HXLh L+vA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:list-unsubscribe:list-subscribe:list-id:precedence :references:message-id:subject:cc:to:from:date:dkim-signature; bh=+2/jVb1kN58UGtCTvPlHVQW5+n1aiYWd2g9lnLxdrmA=; fh=roO8iA3l0dDW0IT5aDqipkVvsY3ZR1WH/wBUf2LXqgA=; b=fUdHlad3r8yNvTzLN9f0lcrAHO4jY1BL01IOlTwPYXa9eg+Zc7G4tc4IWKdtvkr/nn uw2aqBipOBuiSYYf9TFp9mjQj4CaFKd+eWw1ohC3iY9GS7U8dJ++2ckE8tVv19E9h692 CRxXj5RITskY18S3qQwS6nAYEDbxS4wO5jROaMj5E9kvZj6MldrGDPpkvVnjhcx9b5Fq uBUMGj1Sb8zxHaoBe0nhvhKmMGk4lR/RqOewT7zOX07vp3DSRNis3BB0jaIfsjMCZVdT 1m5dORI0nBC9XLmBEO75gCNcNFYdd/EhhjJ4t7LH+CA2CgtfM3AqCk2oDeQnFFcwmPdO mFeg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=GS+ueUOR; arc=pass (i=1 dkim=pass dkdomain=kernel.org); spf=pass (google.com: domain of linux-kernel+bounces-170293-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-170293-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id ay26-20020a170906d29a00b00a599b0901f7si3620933ejb.12.2024.05.06.11.35.27 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 May 2024 11:35:27 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-170293-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=GS+ueUOR; arc=pass (i=1 dkim=pass dkdomain=kernel.org); spf=pass (google.com: domain of linux-kernel+bounces-170293-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-170293-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id CB7741F2434E for ; Mon, 6 May 2024 18:35:26 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2208F158A07; Mon, 6 May 2024 18:35:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="GS+ueUOR" 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 2370F1DFF3; Mon, 6 May 2024 18:35:17 +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=1715020518; cv=none; b=tAPe083QD5qvX+udonFoNWDb0jIcDuQ+OeaneXGnTvdpeQbZCqhHMnWtqyTOzcIoMBlkooqDXr8EVhMRl/MiduUZPlAEMLmztgUGmUYDkuXYIVn7WYZx2tyUBg+oatZtvqdXEbzaYQKXiTDj4AvlNTjSAwDfsg5IrX8qTZz9DN0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715020518; c=relaxed/simple; bh=EJCEu4USxW35SOv0xkyQJk5zUcKRkgxxAOS7xwgdXZs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=B85nmq/TAy75qOhI4oKBI+GyXoNUWNzAAk+pUcJyL1xyKuhqa8t71h9DXoFrlmrjI0JkQkC644MgJitbw1K473g8aMBiDm+sIXOOB5LkMcDc65jWEfdOjamf7XE5PKUgPUx/1CzlW1CoCKY9PcqcCQO7cTrdECILpS2eAGQK4T4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GS+ueUOR; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id C8426C116B1; Mon, 6 May 2024 18:35:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715020517; bh=EJCEu4USxW35SOv0xkyQJk5zUcKRkgxxAOS7xwgdXZs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=GS+ueUORs5m7YW+dTwpnmvxdpAIQQ5nUssdQlK4mzpOaGtdRPjrZOI2mklpXdHdWa HyNVT17rGI/5aikIvknUaOhC2onyCnVm9Hq5b5m3tYe3h40reCCWQKKnQuagXJO44C bIrxou/fvqiPdFUDx5YD6PwgdDxbaesw0wCd7VS2IYp4KVGq+hHSGWltj4pY/jn+lQ /EuYR8uJXDHU9CGcDRVtqH5BEmr2t76pOH+a/IU8x+Xu1z4wROLA5WDt42S3Sd7kVg Ed3v4okJqVlJ4FlBZK8IpEln5RQ5RriaKve3CJzy38L7b2ZGYUJto3103hwGzMSMmc xxZf6g9q/K5YQ== Date: Mon, 6 May 2024 15:35:13 -0300 From: Arnaldo Carvalho de Melo To: Ian Rogers Cc: Peter Zijlstra , Ingo Molnar , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Kan Liang , James Clark , Athira Rajeev , Colin Ian King , nabijaczleweli@nabijaczleweli.xyz, Leo Yan , Song Liu , Ilkka Koskinen , Ben Gainey , K Prateek Nayak , Yanteng Si , Sun Haiyong , Changbin Du , Andi Kleen , Thomas Richter , Masami Hiramatsu , Dima Kogan , zhaimingbing , Paran Lee , Li Dong , Tiezhu Yang , Chengen Du , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 6/7] perf dso: Reference counting related fixes Message-ID: References: <20240504213803.218974-1-irogers@google.com> <20240504213803.218974-7-irogers@google.com> Precedence: bulk X-Mailing-List: linux-kernel@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 Mon, May 06, 2024 at 08:26:02AM -0700, Ian Rogers wrote: > On Mon, May 6, 2024 at 7:44 AM Arnaldo Carvalho de Melo wrote: > > On Sat, May 04, 2024 at 02:38:02PM -0700, Ian Rogers wrote: > > > Ensure gets and puts are better aligned fixing reference couting > > > checking problems. > > > > > > Signed-off-by: Ian Rogers > > > --- > > > tools/perf/util/machine.c | 4 +-- > > > tools/perf/util/map.c | 1 + > > > tools/perf/util/symbol-elf.c | 51 ++++++++++++++++++------------------ > > > 3 files changed, 28 insertions(+), 28 deletions(-) > > > > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > > > index 0b8fb14f5ff6..4a4541a2e887 100644 > > > --- a/tools/perf/util/machine.c > > > +++ b/tools/perf/util/machine.c > > > @@ -683,7 +683,7 @@ static int machine__process_ksymbol_register(struct machine *machine, > > > struct perf_sample *sample __maybe_unused) > > > { > > > struct symbol *sym; > > > - struct dso *dso; > > > + struct dso *dso = NULL; > > > struct map *map = maps__find(machine__kernel_maps(machine), event->ksymbol.addr); > > > int err = 0; > > > > > > @@ -696,7 +696,6 @@ static int machine__process_ksymbol_register(struct machine *machine, > > > } > > > dso__set_kernel(dso, DSO_SPACE__KERNEL); > > > map = map__new2(0, dso); > > > - dso__put(dso); > > > if (!map) { > > > err = -ENOMEM; > > > goto out; > > > @@ -735,6 +734,7 @@ static int machine__process_ksymbol_register(struct machine *machine, > > > dso__insert_symbol(dso, sym); > > > out: > > > map__put(map); > > > + dso__put(dso); > > > return err; > > > } > > > > This seems to match the patch description, good, just aligning the puts > > > > > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c > > > index 117c4bb78b35..e1d14936a60d 100644 > > > --- a/tools/perf/util/map.c > > > +++ b/tools/perf/util/map.c > > > @@ -200,6 +200,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len, > > > dso__set_build_id(dso, dso__bid(header_bid_dso)); > > > dso__set_header_build_id(dso, 1); > > > } > > > + dso__put(header_bid_dso); > > > } > > > dso__put(dso); > > > } > > > > But this is a missing one, so not aligning, but fixing a separate issue, > > i.e. a missing put? Should go on a different patch, probably with a > > Fixes. > > The Fixes would be for a patch in the series - albeit that we've spent Well, if it fixes something that got merged before, it should have the Fixes, please then have it in the patch fixing it: Fixes: f649ed80f3cabbf1 ("perf dsos: Tidy reference counting and locking") > over half a year landing this and a month since that change landed: > https://lore.kernel.org/all/20240410064214.2755936-3-irogers@google.com/ > The issue is that previously a find wouldn't do a get, so per the old > code no put is necessary. Gets were added to finds to cover the race > between a dso being returned from dsos and the item potentially being > removed from dsos. The last patch applied: ⬢[acme@toolbox perf-tools-next]$ git log --oneline -1 ee756ef7491eafd7 (HEAD -> perf-tools-next, perf-tools-next.korg/tmp.perf-tools-next) perf dso: Add reference count checking and accessor functions ⬢[acme@toolbox perf-tools-next]$ Needed this, which I applied and pushed to tmp.perf-tools-next: util/symbol.c: In function ‘dso__load_bfd_symbols’: util/symbol.c:1683:9: error: too few arguments to function ‘dso__set_adjust_symbols’ 1683 | dso__set_adjust_symbols(dso); | ^~~~~~~~~~~~~~~~~~~~~~~ In file included from util/symbol.c:21: util/dso.h:268:20: note: declared here 268 | static inline void dso__set_adjust_symbols(struct dso *dso, bool val) | ^~~~~~~~~~~~~~~~~~~~~~~ make[6]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/tmp.ZWHbQftdN6/util/symbol.o] Error 1 MKDIR /tmp/tmp.ZWHbQftdN6/tests/workloads/ make[6]: *** Waiting for unfinished jobs.... This was updated: - symbols__fixup_end(&dso->symbols, false); - symbols__fixup_duplicate(&dso->symbols); - dso->adjust_symbols = 1; + symbols__fixup_end(dso__symbols(dso), false); + symbols__fixup_duplicate(dso__symbols(dso)); + dso__set_adjust_symbols(dso); But not build tested with BUILD_NONDISTRO and libbfd devel files installed (binutils-devel on fedora), I noticed it using 'make -C tools/perf build-test'. Add the missing argument: symbols__fixup_end(dso__symbols(dso), false); symbols__fixup_duplicate(dso__symbols(dso)); - dso__set_adjust_symbols(dso); + dso__set_adjust_symbols(dso, true);