From mboxrd@z Thu Jan 1 00:00:00 1970 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=-2.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 C1BCEC004EF for ; Tue, 9 Jul 2019 11:22:09 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 967792082A for ; Tue, 9 Jul 2019 11:22:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="LIuYyuS9" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 967792082A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-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=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=eCBUcyxRPRN/ipgIn1Krzt6hONNguyKBexgmL5TV2Uw=; b=LIuYyuS95FOU+9 pGnbqlwPiveZPXP6Tq3MfO+94aD359GJABpqBTLQ2ptmIR9GOOhNNW86reH4Cdh7aswMiyERkQgR1 /6x2xIAe3VStlDQ+kKSuOTER5tFeWa94P3HnddQJP0bqLq6HAvNMgiL+yQzXOKmPMgWN/YjlSQpA9 D44xqoV/X5ymtFdLogZUsvilDfiPAawoWekMjQwpoERuIXxZMPDoIi3ddTsi6sp/sueFqrCrttCGV SMwFIF9valH2M/Qv8g118KF3c7BUzw+fGMlMdt6CWkMdUlH43yec3jGV0CTBDKe3I+6v7Bifzp+HG DdZk5MSJQMR92zBT0FDw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hkoCF-00018U-PY; Tue, 09 Jul 2019 11:22:07 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hkoCD-00018B-DO for linux-arm-kernel@lists.infradead.org; Tue, 09 Jul 2019 11:22:06 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5F0D22B; Tue, 9 Jul 2019 04:22:04 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E0EB93F59C; Tue, 9 Jul 2019 04:22:02 -0700 (PDT) Date: Tue, 9 Jul 2019 12:22:00 +0100 From: Mark Rutland To: Julien Thierry Subject: Re: [PATCH v3 2/9] arm64: perf: Remove PMU locking Message-ID: <20190709112159.GC7929@lakrids.cambridge.arm.com> References: <1562596377-33196-1-git-send-email-julien.thierry@arm.com> <1562596377-33196-3-git-send-email-julien.thierry@arm.com> <20190708150320.GC33099@lakrids.cambridge.arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190709_042205_499906_4666C33E X-CRM114-Status: GOOD ( 19.12 ) 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: peterz@infradead.org, Catalin Marinas , jolsa@redhat.com, will.deacon@arm.com, acme@kernel.org, alexander.shishkin@linux.intel.com, mingo@redhat.com, namhyung@kernel.org, liwei391@huawei.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Jul 08, 2019 at 04:34:01PM +0100, Julien Thierry wrote: > > > On 08/07/2019 16:03, Mark Rutland wrote: > > On Mon, Jul 08, 2019 at 03:32:50PM +0100, Julien Thierry wrote: > >> Since the PMU driver uses direct registers for counter > >> setup/manipulation, locking around these operations is no longer needed. > >> > >> For operations that can be called with interrupts enabled, preemption > >> still needs to be disabled to ensure the programming of the PMU is > >> done on the expected CPU and not migrated mid-programming. > > > > [...] > > > >> static void armv8pmu_start(struct arm_pmu *cpu_pmu) > >> { > >> - unsigned long flags; > >> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events); > >> - > >> - raw_spin_lock_irqsave(&events->pmu_lock, flags); > >> + preempt_disable(); > >> /* Enable all counters */ > >> armv8pmu_pmcr_write(armv8pmu_pmcr_read() | ARMV8_PMU_PMCR_E); > >> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags); > >> + preempt_enable(); > >> } > >> > >> static void armv8pmu_stop(struct arm_pmu *cpu_pmu) > >> { > >> - unsigned long flags; > >> - struct pmu_hw_events *events = this_cpu_ptr(cpu_pmu->hw_events); > >> - > >> - raw_spin_lock_irqsave(&events->pmu_lock, flags); > >> + preempt_disable(); > >> /* Disable all counters */ > >> armv8pmu_pmcr_write(armv8pmu_pmcr_read() & ~ARMV8_PMU_PMCR_E); > >> - raw_spin_unlock_irqrestore(&events->pmu_lock, flags); > >> + preempt_enable(); > >> } > > > > I think that we'd have bigger problems if these could be called in > > preemptible context, since we couldn't guarantee which HW PMU instance > > they'd operate on. > > > > I also thought that the interrupt disable/enable was a hold-over from > > the old days of perf core, and these days all of the synchronous > > operations are held with the pmu ctx lock held (and interrupts > > disabled). > > > > Do you have an example of when these are called with interrupts enabled? > > Or in a preemptible context? > > I must admit that not seeing mention of the pmu_enable/disable callbacks > being called with preemption disabled in perf_event.h, I assumed the > worst. But looking at it, it does look like they are always called > either with interrupts or preemption disabled, and the x86 > implementation doesn't seem to worry about being preempted in those > callbacks. > > I guess I can get rid of the preempt_enable/disable. Yes please! As an aside, this is something I'd like to explicitly annotate on functions so that we could statically and dnyamically test it. For now I'm happy to rely on the semantics documented in perf_event.h. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel