From patchwork Tue Feb 28 11:28:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Aktemur, Tankut Baris" X-Patchwork-Id: 65760 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6D44F3850868 for ; Tue, 28 Feb 2023 11:32:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6D44F3850868 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677583973; bh=ALYKYtUxAnVembkCKkPnBao6hFurO213LvEzpxRyfFk=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=CcvfbkDPHZZWtSAhfM0Al/tDdW1sftGKmTREVMgwUyaTNuhQ2/aT/Y7gh8QYPHdWj ZrqrAe7/VQSTcdW9CeBha2PHpPWh1qwwDkkQaSnvPtDcpxs6CfW7eNmDnbCLDa5GDN A1E0IodlLp7FivP6zWwX46dQGQNnchTbKGsQxJEM= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by sourceware.org (Postfix) with ESMTPS id 713A4384DD17 for ; Tue, 28 Feb 2023 11:31:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 713A4384DD17 X-IronPort-AV: E=McAfee;i="6500,9779,10634"; a="336401834" X-IronPort-AV: E=Sophos;i="5.98,221,1673942400"; d="scan'208";a="336401834" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Feb 2023 03:31:30 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10634"; a="817058381" X-IronPort-AV: E=Sophos;i="5.98,221,1673942400"; d="scan'208";a="817058381" Received: from ultl2604.iul.intel.com (HELO localhost) ([172.28.48.47]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Feb 2023 03:31:28 -0800 To: gdb-patches@sourceware.org Subject: [PATCH 26/26] gdbserver: allow gradually populating and selectively storing a regcache Date: Tue, 28 Feb 2023 12:28:24 +0100 Message-Id: <65c2083374c880b4802a8453dea62b23e2c0dda6.1677582745.git.tankut.baris.aktemur@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Tankut Baris Aktemur via Gdb-patches From: "Aktemur, Tankut Baris" Reply-To: Tankut Baris Aktemur Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" Currently, the regcache can be used by fetching all the registers from the target. For some targets, this could be a costly operation because there is a large number of threads with a large register file each. In this patch, we revise the regcache implementation to allow populating the contents gradually and storing the registers only when they have updated values. To this aim, we introduce a new register status: REG_DIRTY. This status denotes that a register value has been cached and also updated. When invalidating the cache, only the dirty registers are stored to the target. In a typical debug session, it is more likely that only a small subset of the register file has changed. Therefore, selectively storing the registers on targets with many threads and registers can save substantial costs, with respect to storing the whole set. The collect_register function now performs a lazy fetch. If the requested register value is not cached yet, it is requested from the target. The supply_register function updates the status of the supplied register as follows: if the register was not available in the cache, its status becomes REG_VALID, denoting that the value is now cached. If the register is supplied again, it becomes REG_DIRTY. The function that supply the whole register set (supply_regblock and registers_from_string) are also updated to compare the present and new values of each register, so that we can track the register statuses (i.e. dirty or not) properly. Regression-tested on an X86_64 Linux target using the native-gdbserver and native-extended-gdbserver board files. --- gdbserver/regcache.cc | 96 ++++++++++++++++++++++++++++++------ gdbserver/regcache.h | 6 +++ gdbsupport/common-regcache.h | 3 ++ 3 files changed, 91 insertions(+), 14 deletions(-) diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc index cfb68774402..cf020985c31 100644 --- a/gdbserver/regcache.cc +++ b/gdbserver/regcache.cc @@ -63,6 +63,18 @@ regcache::fetch () gdb_assert (this->thread != nullptr); switch_to_thread (this->thread); + /* If there are individually-fetched dirty registers, first + store them, then fetch all. We prefer this to doing + individual fetch for each registers, if needed, because it is + more likely that very few registers are individually-fetched + at this moment and that fetching all in one go is more + efficient than fetching each reg one by one. */ + for (int i = 0; i < tdesc->reg_defs.size (); ++i) + { + if (register_status[i] == REG_DIRTY) + store_inferior_registers (this, i); + } + /* Invalidate all registers, to prevent stale left-overs. */ discard (); fetch_inferior_registers (this, -1); @@ -100,12 +112,17 @@ regcache_invalidate_thread (struct thread_info *thread) void regcache::invalidate () { - if (registers_fetched) + scoped_restore_current_thread restore_thread; + gdb_assert (this->thread != nullptr); + switch_to_thread (this->thread); + + /* Store dirty registers individually. We prefer this to a + store-all, because it is more likely that a small number of + registers have changed. */ + for (int i = 0; i < tdesc->reg_defs.size (); ++i) { - scoped_restore_current_thread restore_thread; - gdb_assert (this->thread != nullptr); - switch_to_thread (this->thread); - store_inferior_registers (this, -1); + if (register_status[i] == REG_DIRTY) + store_inferior_registers (this, i); } discard (); @@ -231,7 +248,8 @@ regcache::registers_to_string (char *buf) unsigned char *regs = registers; for (int i = 0; i < tdesc->reg_defs.size (); ++i) { - if (register_status[i] == REG_VALID) + if (register_status[i] == REG_VALID + || register_status[i] == REG_DIRTY) { bin2hex (regs, buf, register_size (tdesc, i)); buf += register_size (tdesc, i) * 2; @@ -258,9 +276,12 @@ regcache::registers_from_string (const char *buf) if (len > tdesc->registers_size * 2) len = tdesc->registers_size * 2; } - hex2bin (buf, registers, len / 2); - /* All register data have been re-written. Update the statuses. */ - memset (register_status, REG_VALID, tdesc->reg_defs.size ()); + + unsigned char *new_regs = + (unsigned char *) alloca (tdesc->registers_size); + + hex2bin (buf, new_regs, len / 2); + supply_regblock (new_regs); } /* See regcache.h */ @@ -350,7 +371,7 @@ regcache::raw_supply (int n, const void *buf) { memcpy (register_data (n), buf, register_size (tdesc, n)); #ifndef IN_PROCESS_AGENT - set_register_status (n, REG_VALID); + bump_register_status (n); #endif } else @@ -370,7 +391,7 @@ supply_register_zeroed (struct regcache *regcache, int n) memset (regcache->register_data (n), 0, register_size (regcache->tdesc, n)); #ifndef IN_PROCESS_AGENT - regcache->set_register_status (n, REG_VALID); + regcache->bump_register_status (n); #endif } @@ -392,11 +413,26 @@ regcache::supply_regblock (const void *buf) { gdb_assert (buf != nullptr); - memcpy (registers, buf, tdesc->registers_size); #ifndef IN_PROCESS_AGENT - for (int i = 0; i < tdesc->reg_defs.size (); i++) - set_register_status (i, REG_VALID); + /* First, update the statuses. Mark dirty only those that have + changed. */ + unsigned char *regs = registers; + unsigned char *new_regs = (unsigned char *) buf; + for (int i = 0; i < tdesc->reg_defs.size (); ++i) + { + int size = register_size (tdesc, i); + bool first_time = (get_register_status (i) == REG_UNKNOWN); + bool valid = (get_register_status (i) == REG_VALID); + + if (first_time + || (valid && (memcmp (new_regs, regs, size) != 0))) + bump_register_status (i); + + regs += size; + new_regs += size; + } #endif + memcpy (registers, buf, tdesc->registers_size); } #ifndef IN_PROCESS_AGENT @@ -413,6 +449,15 @@ supply_register_by_name (struct regcache *regcache, void collect_register (struct regcache *regcache, int n, void *buf) { +#ifndef IN_PROCESS_AGENT + if (regcache->get_register_status (n) == REG_UNKNOWN) + { + /* This register has not been fetched from the target, yet. + Do it now. */ + fetch_inferior_registers (regcache, n); + } +#endif + regcache->raw_collect (n, buf); } @@ -513,6 +558,29 @@ regcache::set_register_status (int regnum, enum register_status status) #endif } +void +regcache::bump_register_status (int regnum) +{ +#ifndef IN_PROCESS_AGENT + if (register_status == nullptr) + return; +#endif + + switch (get_register_status (regnum)) + { + case REG_UNKNOWN: + set_register_status (regnum, REG_VALID); + break; + + case REG_VALID: + set_register_status (regnum, REG_DIRTY); + break; + + default: + break; + } +} + /* See gdbsupport/common-regcache.h. */ bool diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h index 216044889ec..132709fa71f 100644 --- a/gdbserver/regcache.h +++ b/gdbserver/regcache.h @@ -74,6 +74,12 @@ struct regcache : public reg_buffer_common /* Set the status of register REGNUM to STATUS. */ void set_register_status (int regnum, enum register_status status); + /* Shift the register status "one level" towards REG_DIRTY. + REG_UNKNOWN becomes REG_VALID; + REG_VALID becomes REG_DIRTY; + REG_DIRTY and REG_UNAVAILABLE stay the same. */ + void bump_register_status (int regnum); + /* See gdbsupport/common-regcache.h. */ void raw_supply (int regnum, const void *buf) override; diff --git a/gdbsupport/common-regcache.h b/gdbsupport/common-regcache.h index bf14610f98f..e81238fe7e0 100644 --- a/gdbsupport/common-regcache.h +++ b/gdbsupport/common-regcache.h @@ -31,6 +31,9 @@ enum register_status : signed char /* The register value is valid and cached. */ REG_VALID = 1, + /* The register value is valid, cached, and has been changed. */ + REG_DIRTY = 2, + /* The register value is unavailable. E.g., we're inspecting a traceframe, and this register wasn't collected. Note that this "unavailable" is different from saying the register does not