From patchwork Mon Nov 27 22:08:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Rogers X-Patchwork-Id: 170474 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp3502897vqx; Mon, 27 Nov 2023 14:17:58 -0800 (PST) X-Google-Smtp-Source: AGHT+IGf9VmT7s4S/2ojIDEnNWMfLRr3ASC3ZCojUR+KavUEPEdJ//rSOpXpDGLytKrJf7VxEgG0 X-Received: by 2002:a17:902:d48b:b0:1cf:b8bc:1311 with SMTP id c11-20020a170902d48b00b001cfb8bc1311mr11910362plg.48.1701123478675; Mon, 27 Nov 2023 14:17:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701123478; cv=none; d=google.com; s=arc-20160816; b=SzLf661UVpBigjdtNZ6gHDnI2rPYDpLaqGN4GC07uvqYJ2621mKfUlk1CUlvMf+BJv ThbQgya4iAay2SBs7Q1uA6nlLwZXXu5YmNN5qIqJTIJIulRRz7fGEqMDBGkdq6D+pkSS miRnr9nzAL1lfkAGohxoo2n5q+g93t7eIB8cpypipgQvfVAVQm6DIofs3xSh+YdjjsMY pRdoB1/m52LtmOOZocGQLRwHBgPtRH9A2S3oje7Hral5DTB3johMO+8y1USBoLto3qea 1qprtJJShfNadvMFG5pCaE+/OqKndI/yFhs7myh8tNmbXIiXPM1T6DvJMV2ycUXGXyH4 t7AQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:to:from:subject:references:mime-version :message-id:in-reply-to:date:dkim-signature; bh=vNQKRCmQSB/S/npqwP1oMtmctLGcaju5VMW3BbIURPM=; fh=Io7x3OacpefNRRNKoYAzVE0aWTDwD7FRzyCdD1e3AN4=; b=GgU5W8YC+ItBeR3YXfWzxin335bFgZIRsfvLgg1/qCcDBAyaVPU14JW7LnPrl4wYOQ 0hna8iPTAboLIBYFIk/DXZWR45sUr4yBU3xa3aVXf3wWG09v56YaWdOtYckL9rHuS3uE Xv0Pn7CWfRCx+aBAs+gU/szD0eWQD+mXQByjQ+aLNnyu4MYexFZtm59mq6Z0tstY/Utq VWKxYZHDC+Ul8K/2Rq/uT7Zf9BUNNL2VYmlf79cVZrbjSEBNW8FOKhpV9f5/46xOQ+Bx VnfJmv8odNsbG1qcpqP+XgAOliRyCAOlA5W54QfcgdCYYsFWqSL6bx1fA83FFHgi79Dw hPWw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b="Dkhg5W/V"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id e8-20020a17090301c800b001cfdeb48e92si1460622plh.448.2023.11.27.14.17.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Nov 2023 14:17:58 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b="Dkhg5W/V"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id E8DE8805A7E9; Mon, 27 Nov 2023 14:17:48 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233780AbjK0WRO (ORCPT + 99 others); Mon, 27 Nov 2023 17:17:14 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59970 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233782AbjK0WQg (ORCPT ); Mon, 27 Nov 2023 17:16:36 -0500 Received: from mail-yw1-x114a.google.com (mail-yw1-x114a.google.com [IPv6:2607:f8b0:4864:20::114a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0888A35A7 for ; Mon, 27 Nov 2023 14:11:02 -0800 (PST) Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-5cdde93973aso57471817b3.1 for ; Mon, 27 Nov 2023 14:11:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1701123062; x=1701727862; darn=vger.kernel.org; h=to:from:subject:references:mime-version:message-id:in-reply-to:date :from:to:cc:subject:date:message-id:reply-to; bh=vNQKRCmQSB/S/npqwP1oMtmctLGcaju5VMW3BbIURPM=; b=Dkhg5W/V+VU/hSwOItLalwiFOUQQ8XDuplJjKph2QR7VfmBdeV3Fbd0Fcmkw1plMjv 6sU1DRVDSJNtBxDgJ0BpU9GMyXV0mx9zG0JLVYe78+M5Ib7u6czjswNPBuFDYKZHwowW GkL//NE92k2ujhGS/2XdfX5N7OFkgTcSZ9duyOfpU+AX0TFWZr6xUEVx+1u8zY0Ein45 IlWGmXFqyKtOBtnJcJ9+2rdYP5TVqLROHQ8bp8sGmUY1arAEbOLRugHCK7X/6p1J2tv6 4y3hsmtfIz1I3Vhm5opZzrgUP1/lDRTJmjBgIxGD3qG+MP47Y6eRmVRFz/PJ2vXB+FHt 8WYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701123062; x=1701727862; h=to:from:subject:references:mime-version:message-id:in-reply-to:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vNQKRCmQSB/S/npqwP1oMtmctLGcaju5VMW3BbIURPM=; b=mwzCvM2L7vbqpPZuIGFas47PFZaBr+a8J07W2b7v7jqN52I7SCazB86b2npbviQL0D Wr+8ZltRYnHE1luYyvuKbppw4qGyRvkxuVke/xTqMrVYJq7UEC+VNzK7RZ2poukV9JM5 A/zdJxaom0EcgBZs1Jc47EG3rOV4QA12hdL0iTn1qRkuQEWF+8Kwr5rlSc3UORJY4q34 XqW2Ig7BuRYtHZ5ksGWYN9hMAyC76oPjcSJ+Ow82B4zo6uoCiS/X+IVkZkfEwG4z6EFu dU3Fj/8x0sQYuguGS1SMJDR+2Ipbaa7uqqDkf6lodef7EKiodJgw3DGSRMPyRbwvz1GC zUgw== X-Gm-Message-State: AOJu0Yytlj7usE1hprorrXP/Lj83MnxUDLmeCocs4+2lDB7kvP6KEX7d ZGSGARHB+NbsU4vY/TZlyYXTo4OFgCRr X-Received: from irogers.svl.corp.google.com ([2620:15c:2a3:200:829:6e77:9093:f39b]) (user=irogers job=sendgmr) by 2002:a25:3d44:0:b0:db4:7ac:fea6 with SMTP id k65-20020a253d44000000b00db407acfea6mr394066yba.7.1701123062015; Mon, 27 Nov 2023 14:11:02 -0800 (PST) Date: Mon, 27 Nov 2023 14:08:55 -0800 In-Reply-To: <20231127220902.1315692-1-irogers@google.com> Message-Id: <20231127220902.1315692-44-irogers@google.com> Mime-Version: 1.0 References: <20231127220902.1315692-1-irogers@google.com> X-Mailer: git-send-email 2.43.0.rc1.413.gea7ed67945-goog Subject: [PATCH v5 43/50] perf dsos: Switch backing storage to array from rbtree/list From: Ian Rogers To: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , Nick Terrell , Kan Liang , Andi Kleen , Kajol Jain , Athira Rajeev , Huacai Chen , Masami Hiramatsu , Vincent Whitchurch , "Steinar H. Gunderson" , Liam Howlett , Miguel Ojeda , Colin Ian King , Dmitrii Dolgov <9erthalion6@gmail.com>, Yang Jihong , Ming Wang , James Clark , K Prateek Nayak , Sean Christopherson , Leo Yan , Ravi Bangoria , German Gomez , Changbin Du , Paolo Bonzini , Li Dong , Sandipan Das , liuwenyu , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Guilherme Amadio X-Spam-Status: No, score=-8.4 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Mon, 27 Nov 2023 14:17:49 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783757252759730044 X-GMAIL-MSGID: 1783757252759730044 DSOs were held on a list for fast iteration and in an rbtree for fast finds. Switch to using a lazily sorted array where iteration is just iterating through the array and binary searches are the same complexity as searching the rbtree. The find may need to sort the array first which does increase the complexity, but add operations have lower complexity and overall the complexity should remain about the same. The set name operations on the dso just records that the array is no longer sorted, avoiding complexity in rebalancing the rbtree. Tighter locking discipline is enforced to avoid the array being resorted while long and short names or ids are changed. The array is smaller in size, replacing 6 pointers with 2, and so even with extra allocated space in the array, the array may be 50% unoccupied, the memory saving should be at least 2x. Signed-off-by: Ian Rogers --- tools/perf/util/dso.c | 67 +++++++++------ tools/perf/util/dso.h | 10 +-- tools/perf/util/dsos.c | 188 ++++++++++++++++++++++++++--------------- tools/perf/util/dsos.h | 21 +++-- 4 files changed, 177 insertions(+), 109 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 1b0990507a42..66dc929443ba 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -1240,35 +1240,35 @@ struct dso *machine__findnew_kernel(struct machine *machine, const char *name, return dso; } -static void dso__set_long_name_id(struct dso *dso, const char *name, struct dso_id *id, bool name_allocated) +static void dso__set_long_name_id(struct dso *dso, const char *name, bool name_allocated) { - struct rb_root *root = dso->root; + struct dsos *dsos = dso->dsos; if (name == NULL) return; - if (dso->long_name_allocated) - free((char *)dso->long_name); - - if (root) { - rb_erase(&dso->rb_node, root); + if (dsos) { /* - * __dsos__findnew_link_by_longname_id() isn't guaranteed to - * add it back, so a clean removal is required here. + * Need to avoid re-sorting the dsos breaking by non-atomically + * renaming the dso. */ - RB_CLEAR_NODE(&dso->rb_node); - dso->root = NULL; + down_write(&dsos->lock); } + if (dso->long_name_allocated) + free((char *)dso->long_name); + dso->long_name = name; dso->long_name_len = strlen(name); dso->long_name_allocated = name_allocated; - if (root) - __dsos__findnew_link_by_longname_id(root, dso, NULL, id); + if (dsos) { + dsos->sorted = false; + up_write(&dsos->lock); + } } -static int __dso_id__cmp(struct dso_id *a, struct dso_id *b) +static int __dso_id__cmp(const struct dso_id *a, const struct dso_id *b) { if (a->maj > b->maj) return -1; if (a->maj < b->maj) return 1; @@ -1296,7 +1296,7 @@ static int __dso_id__cmp(struct dso_id *a, struct dso_id *b) return 0; } -bool dso_id__empty(struct dso_id *id) +bool dso_id__empty(const struct dso_id *id) { if (!id) return true; @@ -1304,15 +1304,22 @@ bool dso_id__empty(struct dso_id *id) return !id->maj && !id->min && !id->ino && !id->ino_generation; } -void dso__inject_id(struct dso *dso, struct dso_id *id) +void __dso__inject_id(struct dso *dso, struct dso_id *id) { + struct dsos *dsos = dso->dsos; + + /* dsos write lock held by caller. */ + dso->id.maj = id->maj; dso->id.min = id->min; dso->id.ino = id->ino; dso->id.ino_generation = id->ino_generation; + + if (dsos) + dsos->sorted = false; } -int dso_id__cmp(struct dso_id *a, struct dso_id *b) +int dso_id__cmp(const struct dso_id *a, const struct dso_id *b) { /* * The second is always dso->id, so zeroes if not set, assume passing @@ -1331,20 +1338,34 @@ int dso__cmp_id(struct dso *a, struct dso *b) void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated) { - dso__set_long_name_id(dso, name, NULL, name_allocated); + dso__set_long_name_id(dso, name, name_allocated); } void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated) { + struct dsos *dsos = dso->dsos; + if (name == NULL) return; + if (dsos) { + /* + * Need to avoid re-sorting the dsos breaking by non-atomically + * renaming the dso. + */ + down_write(&dsos->lock); + } if (dso->short_name_allocated) free((char *)dso->short_name); dso->short_name = name; dso->short_name_len = strlen(name); dso->short_name_allocated = name_allocated; + + if (dsos) { + dsos->sorted = false; + up_write(&dsos->lock); + } } int dso__name_len(const struct dso *dso) @@ -1380,7 +1401,7 @@ struct dso *dso__new_id(const char *name, struct dso_id *id) strcpy(dso->name, name); if (id) dso->id = *id; - dso__set_long_name_id(dso, dso->name, id, false); + dso__set_long_name_id(dso, dso->name, false); dso__set_short_name(dso, dso->name, false); dso->symbols = RB_ROOT_CACHED; dso->symbol_names = NULL; @@ -1403,9 +1424,6 @@ struct dso *dso__new_id(const char *name, struct dso_id *id) dso->is_kmod = 0; dso->needs_swap = DSO_SWAP__UNSET; dso->comp = COMP_ID__NONE; - RB_CLEAR_NODE(&dso->rb_node); - dso->root = NULL; - INIT_LIST_HEAD(&dso->node); INIT_LIST_HEAD(&dso->data.open_entry); mutex_init(&dso->lock); refcount_set(&dso->refcnt, 1); @@ -1421,9 +1439,8 @@ struct dso *dso__new(const char *name) void dso__delete(struct dso *dso) { - if (!RB_EMPTY_NODE(&dso->rb_node)) - pr_err("DSO %s is still in rbtree when being deleted!\n", - dso->long_name); + if (dso->dsos) + pr_err("DSO %s is still in rbtree when being deleted!\n", dso->long_name); /* free inlines first, as they reference symbols */ inlines__tree_delete(&dso->inlined_nodes); diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index 1b247eeaa81e..fd500583cd2e 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -146,9 +146,7 @@ struct auxtrace_cache; struct dso { struct mutex lock; - struct list_head node; - struct rb_node rb_node; /* rbtree node sorted by long name */ - struct rb_root *root; /* root of rbtree that rb_node is in */ + struct dsos *dsos; struct rb_root_cached symbols; struct symbol **symbol_names; size_t symbol_names_len; @@ -235,8 +233,8 @@ static inline void dso__set_loaded(struct dso *dso) dso->loaded = true; } -int dso_id__cmp(struct dso_id *a, struct dso_id *b); -bool dso_id__empty(struct dso_id *id); +int dso_id__cmp(const struct dso_id *a, const struct dso_id *b); +bool dso_id__empty(const struct dso_id *id); struct dso *dso__new_id(const char *name, struct dso_id *id); struct dso *dso__new(const char *name); @@ -245,7 +243,7 @@ void dso__delete(struct dso *dso); int dso__cmp_id(struct dso *a, struct dso *b); void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated); void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated); -void dso__inject_id(struct dso *dso, struct dso_id *id); +void __dso__inject_id(struct dso *dso, struct dso_id *id); int dso__name_len(const struct dso *dso); diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c index b7fbfb877ae3..cfc10e1a6802 100644 --- a/tools/perf/util/dsos.c +++ b/tools/perf/util/dsos.c @@ -14,24 +14,30 @@ void dsos__init(struct dsos *dsos) { - INIT_LIST_HEAD(&dsos->head); - dsos->root = RB_ROOT; init_rwsem(&dsos->lock); + + dsos->cnt = 0; + dsos->allocated = 0; + dsos->dsos = NULL; + dsos->sorted = true; } static void dsos__purge(struct dsos *dsos) { - struct dso *pos, *n; - down_write(&dsos->lock); - list_for_each_entry_safe(pos, n, &dsos->head, node) { - RB_CLEAR_NODE(&pos->rb_node); - pos->root = NULL; - list_del_init(&pos->node); - dso__put(pos); + for (unsigned int i = 0; i < dsos->cnt; i++) { + struct dso *dso = dsos->dsos[i]; + + dso__put(dso); + dso->dsos = NULL; } + zfree(&dsos->dsos); + dsos->cnt = 0; + dsos->allocated = 0; + dsos->sorted = true; + up_write(&dsos->lock); } @@ -46,9 +52,8 @@ static int __dsos__for_each_dso(struct dsos *dsos, int (*cb)(struct dso *dso, void *data), void *data) { - struct dso *dso; - - list_for_each_entry(dso, &dsos->head, node) { + for (unsigned int i = 0; i < dsos->cnt; i++) { + struct dso *dso = dsos->dsos[i]; int err; err = cb(dso, data); @@ -119,16 +124,47 @@ static int dso__cmp_short_name(struct dso *a, struct dso *b) return __dso__cmp_short_name(a->short_name, &a->id, b); } +static int dsos__cmp_long_name_id_short_name(const void *va, const void *vb) +{ + const struct dso *a = *((const struct dso **)va); + const struct dso *b = *((const struct dso **)vb); + int rc = strcmp(a->long_name, b->long_name); + + if (!rc) { + rc = dso_id__cmp(&a->id, &b->id); + if (!rc) + rc = strcmp(a->short_name, b->short_name); + } + return rc; +} + /* * Find a matching entry and/or link current entry to RB tree. * Either one of the dso or name parameter must be non-NULL or the * function will not work. */ -struct dso *__dsos__findnew_link_by_longname_id(struct rb_root *root, struct dso *dso, - const char *name, struct dso_id *id) +struct dso *__dsos__findnew_link_by_longname_id(struct dsos *dsos, + struct dso *dso, + const char *name, + struct dso_id *id, + bool write_locked) { - struct rb_node **p = &root->rb_node; - struct rb_node *parent = NULL; + int low = 0, high = dsos->cnt - 1; + + if (!dsos->sorted) { + if (!write_locked) { + up_read(&dsos->lock); + down_write(&dsos->lock); + dso = __dsos__findnew_link_by_longname_id(dsos, dso, name, id, + /*write_locked=*/true); + up_write(&dsos->lock); + down_read(&dsos->lock); + return dso; + } + qsort(dsos->dsos, dsos->cnt, sizeof(struct dso *), + dsos__cmp_long_name_id_short_name); + dsos->sorted = true; + } if (!name) name = dso->long_name; @@ -136,11 +172,11 @@ struct dso *__dsos__findnew_link_by_longname_id(struct rb_root *root, struct dso /* * Find node with the matching name */ - while (*p) { - struct dso *this = rb_entry(*p, struct dso, rb_node); + while (low <= high) { + int mid = (low + high) / 2; + struct dso *this = dsos->dsos[mid]; int rc = __dso__cmp_long_name(name, id, this); - parent = *p; if (rc == 0) { /* * In case the new DSO is a duplicate of an existing @@ -161,56 +197,53 @@ struct dso *__dsos__findnew_link_by_longname_id(struct rb_root *root, struct dso } } if (rc < 0) - p = &parent->rb_left; + high = mid - 1; else - p = &parent->rb_right; - } - if (dso) { - /* Add new node and rebalance tree */ - rb_link_node(&dso->rb_node, parent, p); - rb_insert_color(&dso->rb_node, root); - dso->root = root; + low = mid + 1; } + if (dso) + __dsos__add(dsos, dso); return NULL; } -void __dsos__add(struct dsos *dsos, struct dso *dso) +int __dsos__add(struct dsos *dsos, struct dso *dso) { - list_add_tail(&dso->node, &dsos->head); - __dsos__findnew_link_by_longname_id(&dsos->root, dso, NULL, &dso->id); - /* - * It is now in the linked list, grab a reference, then garbage collect - * this when needing memory, by looking at LRU dso instances in the - * list with atomic_read(&dso->refcnt) == 1, i.e. no references - * anywhere besides the one for the list, do, under a lock for the - * list: remove it from the list, then a dso__put(), that probably will - * be the last and will then call dso__delete(), end of life. - * - * That, or at the end of the 'struct machine' lifetime, when all - * 'struct dso' instances will be removed from the list, in - * dsos__exit(), if they have no other reference from some other data - * structure. - * - * E.g.: after processing a 'perf.data' file and storing references - * to objects instantiated while processing events, we will have - * references to the 'thread', 'map', 'dso' structs all from 'struct - * hist_entry' instances, but we may not need anything not referenced, - * so we might as well call machines__exit()/machines__delete() and - * garbage collect it. - */ - dso__get(dso); + if (dsos->cnt == dsos->allocated) { + unsigned int to_allocate = 2; + struct dso **temp; + + if (dsos->allocated > 0) + to_allocate = dsos->allocated * 2; + temp = realloc(dsos->dsos, sizeof(struct dso *) * to_allocate); + if (!temp) + return -ENOMEM; + dsos->dsos = temp; + dsos->allocated = to_allocate; + } + dsos->dsos[dsos->cnt++] = dso__get(dso); + if (dsos->cnt >= 2 && dsos->sorted) { + dsos->sorted = dsos__cmp_long_name_id_short_name(&dsos->dsos[dsos->cnt - 2], + &dsos->dsos[dsos->cnt - 1]) + <= 0; + } + dso->dsos = dsos; + return 0; } -void dsos__add(struct dsos *dsos, struct dso *dso) +int dsos__add(struct dsos *dsos, struct dso *dso) { + int ret; + down_write(&dsos->lock); - __dsos__add(dsos, dso); + ret = __dsos__add(dsos, dso); up_write(&dsos->lock); + return ret; } -static struct dso *__dsos__findnew_by_longname_id(struct rb_root *root, const char *name, struct dso_id *id) +static struct dso *__dsos__findnew_by_longname_id(struct dsos *dsos, const char *name, + struct dso_id *id, bool write_locked) { - return __dsos__findnew_link_by_longname_id(root, NULL, name, id); + return __dsos__findnew_link_by_longname_id(dsos, NULL, name, id, write_locked); } struct dsos__find_id_cb_args { @@ -231,7 +264,8 @@ static int dsos__find_id_cb(struct dso *dso, void *data) } -static struct dso *__dsos__find_id(struct dsos *dsos, const char *name, struct dso_id *id, bool cmp_short) +static struct dso *__dsos__find_id(struct dsos *dsos, const char *name, struct dso_id *id, + bool cmp_short, bool write_locked) { struct dso *res; @@ -245,7 +279,7 @@ static struct dso *__dsos__find_id(struct dsos *dsos, const char *name, struct d __dsos__for_each_dso(dsos, dsos__find_id_cb, &args); return args.res; } - res = __dsos__findnew_by_longname_id(&dsos->root, name, id); + res = __dsos__findnew_by_longname_id(dsos, name, id, write_locked); return res; } @@ -254,7 +288,7 @@ struct dso *dsos__find(struct dsos *dsos, const char *name, bool cmp_short) struct dso *res; down_read(&dsos->lock); - res = __dsos__find_id(dsos, name, NULL, cmp_short); + res = __dsos__find_id(dsos, name, NULL, cmp_short, /*write_locked=*/false); up_read(&dsos->lock); return res; } @@ -296,8 +330,13 @@ static struct dso *__dsos__addnew_id(struct dsos *dsos, const char *name, struct struct dso *dso = dso__new_id(name, id); if (dso != NULL) { - __dsos__add(dsos, dso); + /* + * The dsos lock is held on entry, so rename the dso before + * adding it to avoid needing to take the dsos lock again to say + * the array isn't sorted. + */ dso__set_basename(dso); + __dsos__add(dsos, dso); } return dso; } @@ -309,10 +348,10 @@ struct dso *__dsos__addnew(struct dsos *dsos, const char *name) static struct dso *__dsos__findnew_id(struct dsos *dsos, const char *name, struct dso_id *id) { - struct dso *dso = __dsos__find_id(dsos, name, id, false); + struct dso *dso = __dsos__find_id(dsos, name, id, false, /*write_locked=*/true); if (dso && dso_id__empty(&dso->id) && !dso_id__empty(id)) - dso__inject_id(dso, id); + __dso__inject_id(dso, id); return dso ? dso : __dsos__addnew_id(dsos, name, id); } @@ -403,18 +442,27 @@ struct dso *dsos__findnew_module_dso(struct dsos *dsos, down_write(&dsos->lock); - dso = __dsos__find_id(dsos, m->name, NULL, /*cmp_short=*/true); + dso = __dsos__find_id(dsos, m->name, NULL, /*cmp_short=*/true, /*write_locked=*/true); + if (dso) { + up_write(&dsos->lock); + return dso; + } + /* + * Failed to find the dso so create it. Change the name before adding it + * to the array, to avoid unnecessary sorts and potential locking + * issues. + */ + dso = dso__new_id(m->name, /*id=*/NULL); if (!dso) { - dso = __dsos__addnew(dsos, m->name); - if (dso == NULL) - goto out_unlock; - - dso__set_module_info(dso, m, machine); - dso__set_long_name(dso, strdup(filename), true); - dso->kernel = DSO_SPACE__KERNEL; + up_write(&dsos->lock); + return NULL; } + dso__set_basename(dso); + dso__set_module_info(dso, m, machine); + dso__set_long_name(dso, strdup(filename), true); + dso->kernel = DSO_SPACE__KERNEL; + __dsos__add(dsos, dso); -out_unlock: up_write(&dsos->lock); return dso; } diff --git a/tools/perf/util/dsos.h b/tools/perf/util/dsos.h index 50bd51523475..c1b3979ad4bd 100644 --- a/tools/perf/util/dsos.h +++ b/tools/perf/util/dsos.h @@ -14,20 +14,22 @@ struct kmod_path; struct machine; /* - * DSOs are put into both a list for fast iteration and rbtree for fast - * long name lookup. + * Collection of DSOs as an array for iteration speed, but sorted for O(n) + * lookup. */ struct dsos { - struct list_head head; - struct rb_root root; /* rbtree root sorted by long name */ struct rw_semaphore lock; + struct dso **dsos; + unsigned int cnt; + unsigned int allocated; + bool sorted; }; void dsos__init(struct dsos *dsos); void dsos__exit(struct dsos *dsos); -void __dsos__add(struct dsos *dsos, struct dso *dso); -void dsos__add(struct dsos *dsos, struct dso *dso); +int __dsos__add(struct dsos *dsos, struct dso *dso); +int dsos__add(struct dsos *dsos, struct dso *dso); struct dso *__dsos__addnew(struct dsos *dsos, const char *name); struct dso *dsos__find(struct dsos *dsos, const char *name, bool cmp_short); @@ -35,8 +37,11 @@ struct dso *dsos__findnew_id(struct dsos *dsos, const char *name, struct dso_id bool dsos__read_build_ids(struct dsos *dsos, bool with_hits); -struct dso *__dsos__findnew_link_by_longname_id(struct rb_root *root, struct dso *dso, - const char *name, struct dso_id *id); +struct dso *__dsos__findnew_link_by_longname_id(struct dsos *dsos, + struct dso *dso, + const char *name, + struct dso_id *id, + bool write_locked); size_t dsos__fprintf_buildid(struct dsos *dsos, FILE *fp, bool (skip)(struct dso *dso, int parm), int parm);