From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751506AbdASKB4 (ORCPT ); Thu, 19 Jan 2017 05:01:56 -0500 Received: from mail.skyhub.de ([78.46.96.112]:39571 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751360AbdASKBy (ORCPT ); Thu, 19 Jan 2017 05:01:54 -0500 Date: Thu, 19 Jan 2017 11:01:48 +0100 From: Borislav Petkov To: Suravee Suthikulpanit Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, joro@8bytes.org, peterz@infradead.org, mingo@redhat.com Subject: Re: [PATCH v8 3/9] perf/amd/iommu: Misc fix up perf_iommu_read Message-ID: <20170119100148.vvmi7m2kpmtd6ylz@pd.tnic> References: <1484551416-5440-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1484551416-5440-4-git-send-email-Suravee.Suthikulpanit@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1484551416-5440-4-git-send-email-Suravee.Suthikulpanit@amd.com> User-Agent: NeoMutt/20161014 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > Subject: Re: [PATCH v8 3/9] perf/amd/iommu: Misc fix up perf_iommu_read Please be a bit more thorough when writing your commit messages. What is a "misc fix up"? Perhaps it is ok for a quick'n'dirty local patch but not when it is for upstream. Also, function names end with "()" to denote they're functions. On Mon, Jan 16, 2017 at 01:23:30AM -0600, Suravee Suthikulpanit wrote: > * Fix overflow handling since u64 delta would lose the MSB sign bit. > * Remove unnecessary local64_cmpxchg(). > * Coding style and make use of GENMASK_ULL macro. That last one doesn't read like a sentence. > Cc: Peter Zijlstra > Cc: Borislav Petkov > Signed-off-by: Suravee Suthikulpanit > --- > arch/x86/events/amd/iommu.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c > index 1aa25d8..3f1c18a 100644 > --- a/arch/x86/events/amd/iommu.c > +++ b/arch/x86/events/amd/iommu.c > @@ -320,9 +320,8 @@ static void perf_iommu_start(struct perf_event *event, int flags) > > static void perf_iommu_read(struct perf_event *event) > { > - u64 count = 0ULL; > - u64 prev_raw_count = 0ULL; > - u64 delta = 0ULL; > + u64 count, prev; > + s64 delta; > struct hw_perf_event *hwc = &event->hw; > > amd_iommu_pc_get_set_reg_val(_GET_DEVID(event), > @@ -330,18 +329,20 @@ static void perf_iommu_read(struct perf_event *event) > IOMMU_PC_COUNTER_REG, &count, false); > > /* IOMMU pc counter register is only 48 bits */ > - count &= 0xFFFFFFFFFFFFULL; > + count &= GENMASK_ULL(48, 0); GENMASK_ULL(48, 0) gives 0x1ffffffffffff, however. Which is 49 bits. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.