Received: by 10.213.65.68 with SMTP id h4csp3369920imn; Tue, 3 Apr 2018 03:50:30 -0700 (PDT) X-Google-Smtp-Source: AIpwx4/WZODusKVaDU6lzUfLGD8DPXz2x+SavVS54yEh9PjLGKklPb1HFcAtXZ4/THDbGt4RjbQg X-Received: by 10.101.78.8 with SMTP id r8mr1580584pgt.343.1522752630822; Tue, 03 Apr 2018 03:50:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1522752630; cv=none; d=google.com; s=arc-20160816; b=l4+yXR2+SfdswOnqvz+4oe9Mb7rzJgiYbNx1i751Xw9UrEF5rWRw8k6hWm8p5u+LGk sbz8nJqLrzGddQrfvKLoNmMqm1A2waOg1lNAaSyDHb0CROBrkc/0QSj1ftHnQsvxfWsG yuJfkklL+R/AJIANWdpQou7dhH7sEFAHrIuprXuWwoy9FfspMKvvotxIE2nmi2SjYMro 0dSTbwalkFlGi1jT2/ay7cYA4aT9SIgGq4Ry0oBC0OOSmsaYwYduZ7UzvGMGQRGdP0xN woWKaDtmzdVfXI95gQC+0oYXEp3tVrMbqH/vtDo1aa7079ieBIMrqWYvhQBe1UOVYB7J lyVg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature:arc-authentication-results; bh=riuxwibuqQXw6r4X1bcUCzmOmYJYm0qQw1U+AfB+aHM=; b=MSZd3t4ECzIXDetmRffcuLF1Xr5xe1vmLxq1qlDQK7pcw5CE9z1RpckgGja5ueI5YS 4kOQ5a3UKj201nyMeB2X4sf58W7qsJ/Jd9tNTq1XhwZLtpBaQV9dBv+utwVS3OhRlrvl /E8xP5jsz45iAQ94a5g1yBSplsPAd56SleMej90XNTs3uXWpeN/RMyfjah72h77i5ICM q03RFnrvjTqDxW67ahw93e//wnZAXVDMySgCxZKbaa7JnoRjB3042PEygaTpSUPym4Dk 7QTBPuvCLSwnuFHkNRuMz7BABAfsK5DBgNwvbZEtld/JNZNzcK+i5xPAMeBiD5ziwIPl H4fw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b=FEC0voir; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id g66si416383pgc.624.2018.04.03.03.49.46; Tue, 03 Apr 2018 03:50:30 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b=FEC0voir; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755191AbeDCKqD (ORCPT + 99 others); Tue, 3 Apr 2018 06:46:03 -0400 Received: from mail-wm0-f46.google.com ([74.125.82.46]:38107 "EHLO mail-wm0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754893AbeDCKqC (ORCPT ); Tue, 3 Apr 2018 06:46:02 -0400 Received: by mail-wm0-f46.google.com with SMTP id i3so10424149wmf.3 for ; Tue, 03 Apr 2018 03:46:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=riuxwibuqQXw6r4X1bcUCzmOmYJYm0qQw1U+AfB+aHM=; b=FEC0voirieiriwnzeRyzv37by6aEo/Q+IK1Gpfv4IztMBoLCq4CnR1DBrdpz9IfQCJ A5IlsCmtxzF1vwEEbhrtJfnVIhCsl0FM0ZwHpEmcBj0mqB86nC7c10Yi3AEs+ZLd7KqS WPZHzVQxnBmrOZpldXhAZXHME4rPjdnxVkTWG3M2UzuMQWv+xcQJutKDGW6veWLf6eqC Wt3uSaqmbQAErXnU10fStW8JhLhpYIpP2ZzDrYpFyjpJtR3NG4gpIPNQPljyMSOpBSTj v8NrFY6DpwXY49siOpeyM7qnPxQOF6B9GqYDn5ryarnhIgzgxuVyoh4HmwvR/Z8HNNZM tj9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=riuxwibuqQXw6r4X1bcUCzmOmYJYm0qQw1U+AfB+aHM=; b=C65Odt2qEcjibHumEF9RagQQ4501ovjEXiFYR3qpr9QJUObnV+h4t/r0tMXLwjuTNF X55ynI9PCy8K/bG6XLnAaeblewucDB4E+Yogf3Z8NwXvprRR7SaZvhF/lHcjLwiybOq/ 80iGYy86atutG3kxsStHPcaF0/fH6gzgUIJKbNWPL4vRNwJo22xZS3XrqGIJvapKuLLq gV2PUTSS6+YC65WJgh5hYMFydOmH7duhOTxHShyXU1H1FAOyAnld8riMtlveBIThVA1f U1JgcpkjzBKzon7j1O6TaL3n+oEWXVE6sauLjRaEpn6HbLKxavAt8eEwXPLSmXnr5oIT a3NQ== X-Gm-Message-State: AElRT7E7EZbGJ6K4/cNNFCeCm1YfApB/srhx6Kafj56L3vQkO9UPiAFZ 97UmzJwmiUjNB9rs+zOCjbU= X-Received: by 10.28.226.138 with SMTP id z132mr3389462wmg.101.1522752361340; Tue, 03 Apr 2018 03:46:01 -0700 (PDT) Received: from gmail.com (2E8B0CD5.catv.pool.telekom.hu. [46.139.12.213]) by smtp.gmail.com with ESMTPSA id 1sm4573484wrz.68.2018.04.03.03.46.00 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 03 Apr 2018 03:46:00 -0700 (PDT) Date: Tue, 3 Apr 2018 12:45:58 +0200 From: Ingo Molnar To: Stephane Eranian Cc: linux-kernel@vger.kernel.org, acme@redhat.com, peterz@infradead.org, mingo@elte.hu, ak@linux.intel.com, kan.liang@intel.com, jolsa@redhat.com Subject: Re: [PATCH] perf/x86/intel: move regs->flags EXACT bit init Message-ID: <20180403104558.ejcyn4hskpfe6v6n@gmail.com> References: <1522741649-13904-1-git-send-email-eranian@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1522741649-13904-1-git-send-email-eranian@google.com> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Stephane Eranian wrote: > This patch removes a redundant store on regs->flags introduced > by commit: > > 71eb9ee9596d ("perf/x86/intel: Fix linear IP of PEBS real_ip on Haswell and later CPUs") > > We were clearing the PERF_EFLAGS_EXACT but it was overwritten by > regs->flags = pebs->flags later on. > > The PERF_EFLAGS_EXACT is a software flag using bit 3 of regs->flags. > X86 marks this bit as Reserved. To make sure this bit is zero before > we do any IP processing, we clear it explicitly. > > Patch also removes the following assignment: > regs->flags = pebs->flags | (regs->flags & PERF_EFLAGS_VM); > > Because there is no regs->flags to preserve anymore because > set_linear_ip() is not called until later. > > Patch also clarifies comment for intel_pmu_pebs_fixup_ip(). > > Signed-off-by: Stephane Eranian > --- > arch/x86/events/intel/ds.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c > index da6780122786..41b44a4fff51 100644 > --- a/arch/x86/events/intel/ds.c > +++ b/arch/x86/events/intel/ds.c > @@ -1153,7 +1153,6 @@ static void setup_pebs_sample_data(struct perf_event *event, > if (pebs == NULL) > return; > > - regs->flags &= ~PERF_EFLAGS_EXACT; > sample_type = event->attr.sample_type; > dsrc = sample_type & PERF_SAMPLE_DATA_SRC; > > @@ -1197,7 +1196,13 @@ static void setup_pebs_sample_data(struct perf_event *event, > * and PMI. > */ > *regs = *iregs; > - regs->flags = pebs->flags; > + > + /* > + * initialize regs_>flags from pebs > + * clear exact bit (which uses Reserved bit 3), > + * i.e, do not rely on it being zero. > + */ > + regs->flags = pebs->flags & ~PERF_EFLAGS_EXACT; Please use consistent capitalization and spelling to make comments more readable: s/initialize /Initialize s/pebs /PEBS s/i.e /i.e. Also, please put a comma after 'PEBS', to make it more clear what the sentence says. > - /* Haswell and later have the eventing IP, so use it: */ > + /* Haswell and later have the eventing IP, so use it */ So that's a step backwards in readability ... > - /* Otherwise use PEBS off-by-1 IP: */ > + /* Otherwise use PEBS off-by-1 IP */ Ditto. > set_linear_ip(regs, pebs->ip); > > - /* ... and try to fix it up using the LBR entries: */ > + /* > + * ... and try to fix it up using the LBR entries > + * if successful, regs->ip modified and regs patch > + * via set_linear_ip() > + */ > if (intel_pmu_pebs_fixup_ip(regs)) > regs->flags |= PERF_EFLAGS_EXACT; And it's unclear to me what this tries to say: "try to fix it up using the LBR entries if successful", or: "try to fix it up using the LBR entries, and if successful, regs->ip modified and regs patch via set_linear_ip()", ? Please improve readability. Thanks, Ingo