From patchwork Thu Nov 19 15:24:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leo Yan X-Patchwork-Id: 11917909 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EE0E0C6369E for ; Thu, 19 Nov 2020 15:25:59 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 57900246EE for ; Thu, 19 Nov 2020 15:25:59 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="SB2T1uVE"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="Cj0ZQP6M" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 57900246EE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:MIME-Version:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:References:In-Reply-To:Message-Id:Date:Subject:To: From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=skhn/ZD8YiK/Vtyv791nvuqDEWaiG8kh90MfpqbXJDs=; b=SB2T1uVE3HNB6L8Wa34OeTZFRz JNVfG0bXy+UH44OJ6631jeNeyD6AXVZtlXwzOmxm9zPx4C+I+Aczx67YEa3od3m+u1RggMRYssjKV PrC36dl8ZoLb1WmR5nFYPWHZQMGxycR0beTEIFpnC5n0ql+vofTFWXq/7z6ehJRohD69HtCXautI9 Gp/4vY6eLG0TpGYUYvK50lOYhTNTpTvOO9lZI7mt0FWWdFYdvEQZ7kNnvTvMWzspbSdMzz/ht1YVj 0xw8PfGbANjywdZwmLkU4DOX7TQvqdHv6brV0Ocue/ZICV8YH1+SlY68L61t3if/sik/1i/K1KKBJ IwEVnMnA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kfloD-0004DP-MW; Thu, 19 Nov 2020 15:25:17 +0000 Received: from mail-pg1-x531.google.com ([2607:f8b0:4864:20::531]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kflnx-00045c-Ll for linux-arm-kernel@lists.infradead.org; Thu, 19 Nov 2020 15:25:06 +0000 Received: by mail-pg1-x531.google.com with SMTP id w4so4466933pgg.13 for ; Thu, 19 Nov 2020 07:24:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=IQi7P+ZwB9nMZCJaFMbbRNgVda79+plJyZgUnqm8llk=; b=Cj0ZQP6MGw4sJTwEZpnTV4ycImAUrughmpij/F/v2DfQxj2fYaqUjQsUidUfk0TLJo wXPA52jiGL+mtYomQaIXyXPKqu2u3fiF00MtlkQavPEJ6/O+veyMpiTanBg9OgBjvWRe Q89UMOzfmeedB63zPBQe28iXdBFxd1u0JYWC3CZU7RArcJIejxjcQEO3C7Ivvo+Xrfxq f1iV8BIxEvhVFlBNWPKAMa8RUDolLqW4nHjmIrOQvo5k7A+q2/b7p13QpdCLu76nGzbb pjEV4rAAoL46XKQJhAgU3MF5txLkxgTimXU/fSdKSbN/pJJxI4ct7YKr/xBN183eEgCV By1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=IQi7P+ZwB9nMZCJaFMbbRNgVda79+plJyZgUnqm8llk=; b=Kssx5WpAW9KTOPOgjLhsQpy6kADHYNsrJK8Cis/WpErSsVe6B1+8XeNtaaW0B9xb31 beqWsjgk66uEOTXVIcVAyPIbGsyax3GnEBvuUfFzPc05yeei+166tlBAKNcA4Q0pTpJ3 NxfFo5kMPKLdrB364wUwXm2KR6LGpQ02JilcSaiKyAU3LgHrJIA9v0YNcZUbm+fsJl2b 3I4cGcfqrM2Ms1g4vwGNjAO3Jc++eB6cshLas61bwlBtrJLm9YSF8riD39VeS8TrPsID Y5CNw4CFIOewyNIi9gjfAjX5My+DOaLrDBy1HXb8CkRKojaLn+6F2WiiQ+GTF3tWyFND QY/Q== X-Gm-Message-State: AOAM530fHCsRuce/y20a814wgJyIdQBEtU6lqBjb3rPJhJ+zyCoQqdNG leb875+OmREHHly0DHzloF65GA== X-Google-Smtp-Source: ABdhPJyqrqwjmaWC3EGNuSI6nRNCQ4oWwiAQymlG4TNRjuIAV4Ai4dogTlodm0qaZCIeqhvUyfDl/g== X-Received: by 2002:a17:90a:7e0f:: with SMTP id i15mr5168977pjl.77.1605799497022; Thu, 19 Nov 2020 07:24:57 -0800 (PST) Received: from localhost ([45.137.216.7]) by smtp.gmail.com with ESMTPSA id a3sm121760pfo.46.2020.11.19.07.24.55 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 19 Nov 2020 07:24:56 -0800 (PST) From: Leo Yan To: Arnaldo Carvalho de Melo , Andre Przywara , Dave Martin , James Clark , Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Al Grant , Wei Li , John Garry , Will Deacon , Mathieu Poirier , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: [PATCH v9 01/16] perf arm-spe: Refactor printing string to buffer Date: Thu, 19 Nov 2020 23:24:26 +0800 Message-Id: <20201119152441.6972-2-leo.yan@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20201119152441.6972-1-leo.yan@linaro.org> References: <20201119152441.6972-1-leo.yan@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201119_102501_923583_187E4AAB X-CRM114-Status: GOOD ( 30.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Leo Yan MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org When outputs strings to the decoding buffer with function snprintf(), SPE decoder needs to detects if any error returns from snprintf() and if so needs to directly bail out. If snprintf() returns success, it needs to update buffer pointer and reduce the buffer length so can continue to output the next string into the consequent memory space. This complex logics are spreading in the function arm_spe_pkt_desc() so there has many duplicate codes for handling error detecting, increment buffer pointer and decrement buffer size. To avoid the duplicate code, this patch introduces a new helper function arm_spe_pkt_out_string() which is used to wrap up the complex logics, and it's used by the caller arm_spe_pkt_desc(). This patch moves the variable 'blen' as the function's local variable so allows to remove the unnecessary braces and improve the readability. This patch simplifies the return value for arm_spe_pkt_desc(): '0' means success and other values mean an error has occurred. To realize this, it relies on arm_spe_pkt_out_string()'s parameter 'err', the 'err' is a cumulative value, returns its final value if printing buffer is called for one time or multiple times. Finally, the error is handled in a central place, rather than directly bailing out in switch-cases, it returns error at the end of arm_spe_pkt_desc(). This patch changes the caller arm_spe_dump() to respect the updated return value semantics of arm_spe_pkt_desc(). Suggested-by: Dave Martin Signed-off-by: Leo Yan Reviewed-by: Andre Przywara Reviewed-by: Dave Martin --- .../arm-spe-decoder/arm-spe-pkt-decoder.c | 302 +++++++++--------- tools/perf/util/arm-spe.c | 2 +- 2 files changed, 151 insertions(+), 153 deletions(-) diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c index 671a4763fb47..fbededc1bcd4 100644 --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c @@ -9,6 +9,7 @@ #include #include #include +#include #include "arm-spe-pkt-decoder.h" @@ -258,192 +259,189 @@ int arm_spe_get_packet(const unsigned char *buf, size_t len, return ret; } +static int arm_spe_pkt_out_string(int *err, char **buf_p, size_t *blen, + const char *fmt, ...) +{ + va_list ap; + int ret; + + /* Bail out if any error occurred */ + if (err && *err) + return *err; + + va_start(ap, fmt); + ret = vsnprintf(*buf_p, *blen, fmt, ap); + va_end(ap); + + if (ret < 0) { + if (err && !*err) + *err = ret; + + /* + * A return value of *blen or more means that the output was + * truncated and the buffer is overrun. + */ + } else if ((size_t)ret >= *blen) { + (*buf_p)[*blen - 1] = '\0'; + + /* + * Set *err to 'ret' to avoid overflow if tries to + * fill this buffer sequentially. + */ + if (err && !*err) + *err = ret; + } else { + *buf_p += ret; + *blen -= ret; + } + + return ret; +} + int arm_spe_pkt_desc(const struct arm_spe_pkt *packet, char *buf, size_t buf_len) { - int ret, ns, el, idx = packet->index; + int ns, el, idx = packet->index; unsigned long long payload = packet->payload; const char *name = arm_spe_pkt_name(packet->type); + char *buf_orig = buf; + size_t blen = buf_len; + int err = 0; switch (packet->type) { case ARM_SPE_BAD: case ARM_SPE_PAD: case ARM_SPE_END: - return snprintf(buf, buf_len, "%s", name); - case ARM_SPE_EVENTS: { - size_t blen = buf_len; - - ret = 0; - ret = snprintf(buf, buf_len, "EV"); - buf += ret; - blen -= ret; - if (payload & 0x1) { - ret = snprintf(buf, buf_len, " EXCEPTION-GEN"); - buf += ret; - blen -= ret; - } - if (payload & 0x2) { - ret = snprintf(buf, buf_len, " RETIRED"); - buf += ret; - blen -= ret; - } - if (payload & 0x4) { - ret = snprintf(buf, buf_len, " L1D-ACCESS"); - buf += ret; - blen -= ret; - } - if (payload & 0x8) { - ret = snprintf(buf, buf_len, " L1D-REFILL"); - buf += ret; - blen -= ret; - } - if (payload & 0x10) { - ret = snprintf(buf, buf_len, " TLB-ACCESS"); - buf += ret; - blen -= ret; - } - if (payload & 0x20) { - ret = snprintf(buf, buf_len, " TLB-REFILL"); - buf += ret; - blen -= ret; - } - if (payload & 0x40) { - ret = snprintf(buf, buf_len, " NOT-TAKEN"); - buf += ret; - blen -= ret; - } - if (payload & 0x80) { - ret = snprintf(buf, buf_len, " MISPRED"); - buf += ret; - blen -= ret; - } + arm_spe_pkt_out_string(&err, &buf, &blen, "%s", name); + break; + case ARM_SPE_EVENTS: + arm_spe_pkt_out_string(&err, &buf, &blen, "EV"); + + if (payload & 0x1) + arm_spe_pkt_out_string(&err, &buf, &blen, " EXCEPTION-GEN"); + if (payload & 0x2) + arm_spe_pkt_out_string(&err, &buf, &blen, " RETIRED"); + if (payload & 0x4) + arm_spe_pkt_out_string(&err, &buf, &blen, " L1D-ACCESS"); + if (payload & 0x8) + arm_spe_pkt_out_string(&err, &buf, &blen, " L1D-REFILL"); + if (payload & 0x10) + arm_spe_pkt_out_string(&err, &buf, &blen, " TLB-ACCESS"); + if (payload & 0x20) + arm_spe_pkt_out_string(&err, &buf, &blen, " TLB-REFILL"); + if (payload & 0x40) + arm_spe_pkt_out_string(&err, &buf, &blen, " NOT-TAKEN"); + if (payload & 0x80) + arm_spe_pkt_out_string(&err, &buf, &blen, " MISPRED"); if (idx > 1) { - if (payload & 0x100) { - ret = snprintf(buf, buf_len, " LLC-ACCESS"); - buf += ret; - blen -= ret; - } - if (payload & 0x200) { - ret = snprintf(buf, buf_len, " LLC-REFILL"); - buf += ret; - blen -= ret; - } - if (payload & 0x400) { - ret = snprintf(buf, buf_len, " REMOTE-ACCESS"); - buf += ret; - blen -= ret; - } + if (payload & 0x100) + arm_spe_pkt_out_string(&err, &buf, &blen, " LLC-ACCESS"); + if (payload & 0x200) + arm_spe_pkt_out_string(&err, &buf, &blen, " LLC-REFILL"); + if (payload & 0x400) + arm_spe_pkt_out_string(&err, &buf, &blen, " REMOTE-ACCESS"); } - if (ret < 0) - return ret; - blen -= ret; - return buf_len - blen; - } + break; case ARM_SPE_OP_TYPE: switch (idx) { - case 0: return snprintf(buf, buf_len, "%s", payload & 0x1 ? - "COND-SELECT" : "INSN-OTHER"); - case 1: { - size_t blen = buf_len; + case 0: + arm_spe_pkt_out_string(&err, &buf, &blen, + payload & 0x1 ? "COND-SELECT" : "INSN-OTHER"); + break; + case 1: + arm_spe_pkt_out_string(&err, &buf, &blen, + payload & 0x1 ? "ST" : "LD"); - if (payload & 0x1) - ret = snprintf(buf, buf_len, "ST"); - else - ret = snprintf(buf, buf_len, "LD"); - buf += ret; - blen -= ret; if (payload & 0x2) { - if (payload & 0x4) { - ret = snprintf(buf, buf_len, " AT"); - buf += ret; - blen -= ret; - } - if (payload & 0x8) { - ret = snprintf(buf, buf_len, " EXCL"); - buf += ret; - blen -= ret; - } - if (payload & 0x10) { - ret = snprintf(buf, buf_len, " AR"); - buf += ret; - blen -= ret; - } + if (payload & 0x4) + arm_spe_pkt_out_string(&err, &buf, &blen, " AT"); + if (payload & 0x8) + arm_spe_pkt_out_string(&err, &buf, &blen, " EXCL"); + if (payload & 0x10) + arm_spe_pkt_out_string(&err, &buf, &blen, " AR"); } else if (payload & 0x4) { - ret = snprintf(buf, buf_len, " SIMD-FP"); - buf += ret; - blen -= ret; - } - if (ret < 0) - return ret; - blen -= ret; - return buf_len - blen; - } - case 2: { - size_t blen = buf_len; - - ret = snprintf(buf, buf_len, "B"); - buf += ret; - blen -= ret; - if (payload & 0x1) { - ret = snprintf(buf, buf_len, " COND"); - buf += ret; - blen -= ret; - } - if (payload & 0x2) { - ret = snprintf(buf, buf_len, " IND"); - buf += ret; - blen -= ret; - } - if (ret < 0) - return ret; - blen -= ret; - return buf_len - blen; + arm_spe_pkt_out_string(&err, &buf, &blen, " SIMD-FP"); } - default: return 0; + break; + case 2: + arm_spe_pkt_out_string(&err, &buf, &blen, "B"); + + if (payload & 0x1) + arm_spe_pkt_out_string(&err, &buf, &blen, " COND"); + if (payload & 0x2) + arm_spe_pkt_out_string(&err, &buf, &blen, " IND"); + + break; + default: + /* Unknown index */ + err = -1; + break; } + break; case ARM_SPE_DATA_SOURCE: case ARM_SPE_TIMESTAMP: - return snprintf(buf, buf_len, "%s %lld", name, payload); + arm_spe_pkt_out_string(&err, &buf, &blen, "%s %lld", name, payload); + break; case ARM_SPE_ADDRESS: switch (idx) { case 0: - case 1: ns = !!(packet->payload & NS_FLAG); + case 1: + ns = !!(packet->payload & NS_FLAG); el = (packet->payload & EL_FLAG) >> 61; payload &= ~(0xffULL << 56); - return snprintf(buf, buf_len, "%s 0x%llx el%d ns=%d", + arm_spe_pkt_out_string(&err, &buf, &blen, + "%s 0x%llx el%d ns=%d", (idx == 1) ? "TGT" : "PC", payload, el, ns); - case 2: return snprintf(buf, buf_len, "VA 0x%llx", payload); - case 3: ns = !!(packet->payload & NS_FLAG); + break; + case 2: + arm_spe_pkt_out_string(&err, &buf, &blen, + "VA 0x%llx", payload); + break; + case 3: + ns = !!(packet->payload & NS_FLAG); payload &= ~(0xffULL << 56); - return snprintf(buf, buf_len, "PA 0x%llx ns=%d", - payload, ns); - default: return 0; + arm_spe_pkt_out_string(&err, &buf, &blen, + "PA 0x%llx ns=%d", payload, ns); + break; + default: + /* Unknown index */ + err = -1; + break; } + break; case ARM_SPE_CONTEXT: - return snprintf(buf, buf_len, "%s 0x%lx el%d", name, - (unsigned long)payload, idx + 1); - case ARM_SPE_COUNTER: { - size_t blen = buf_len; - - ret = snprintf(buf, buf_len, "%s %d ", name, - (unsigned short)payload); - buf += ret; - blen -= ret; + arm_spe_pkt_out_string(&err, &buf, &blen, "%s 0x%lx el%d", + name, (unsigned long)payload, idx + 1); + break; + case ARM_SPE_COUNTER: + arm_spe_pkt_out_string(&err, &buf, &blen, "%s %d ", name, + (unsigned short)payload); switch (idx) { - case 0: ret = snprintf(buf, buf_len, "TOT"); break; - case 1: ret = snprintf(buf, buf_len, "ISSUE"); break; - case 2: ret = snprintf(buf, buf_len, "XLAT"); break; - default: ret = 0; + case 0: + arm_spe_pkt_out_string(&err, &buf, &blen, "TOT"); + break; + case 1: + arm_spe_pkt_out_string(&err, &buf, &blen, "ISSUE"); + break; + case 2: + arm_spe_pkt_out_string(&err, &buf, &blen, "XLAT"); + break; + default: + break; } - if (ret < 0) - return ret; - blen -= ret; - return buf_len - blen; - } + break; default: + /* Unknown packet type */ + err = -1; break; } - return snprintf(buf, buf_len, "%s 0x%llx (%d)", - name, payload, packet->index); + /* Output raw data if detect any error */ + if (err) { + err = 0; + arm_spe_pkt_out_string(&err, &buf_orig, &buf_len, "%s 0x%llx (%d)", + name, payload, packet->index); + } + + return err; } diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c index 3882a5360ada..8901a1656a41 100644 --- a/tools/perf/util/arm-spe.c +++ b/tools/perf/util/arm-spe.c @@ -113,7 +113,7 @@ static void arm_spe_dump(struct arm_spe *spe __maybe_unused, if (ret > 0) { ret = arm_spe_pkt_desc(&packet, desc, ARM_SPE_PKT_DESC_MAX); - if (ret > 0) + if (!ret) color_fprintf(stdout, color, " %s\n", desc); } else { color_fprintf(stdout, color, " Bad packet!\n");