From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965210AbcIPTwM (ORCPT ); Fri, 16 Sep 2016 15:52:12 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:44134 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935460AbcIPTwD (ORCPT ); Fri, 16 Sep 2016 15:52:03 -0400 DMARC-Filter: OpenDMARC Filter v1.3.1 smtp.codeaurora.org B1AB36159A Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=pass smtp.mailfrom=nleeder@codeaurora.org Subject: Re: [PATCH v4 2/2] soc: qcom: add l2 cache perf events driver To: Mark Rutland References: <1472576493-26382-1-git-send-email-nleeder@codeaurora.org> <1472576493-26382-3-git-send-email-nleeder@codeaurora.org> <20160901163051.GA6731@leverpostej> <27f91809-f968-e278-7887-d0f9af275502@codeaurora.org> <20160916164011.GB3179@leverpostej> Cc: Catalin Marinas , Will Deacon , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Mark Langsdorf , Mark Salter , Jon Masters , Timur Tabi , cov@codeaurora.org, nleeder@codeaurora.org From: Neil Leeder Message-ID: Date: Fri, 16 Sep 2016 15:51:58 -0400 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160916164011.GB3179@leverpostej> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 9/16/2016 12:40 PM, Mark Rutland wrote: > On Fri, Sep 16, 2016 at 11:33:39AM -0400, Neil Leeder wrote: [...] >> On 9/1/2016 12:30 PM, Mark Rutland wrote: >>> On Tue, Aug 30, 2016 at 01:01:33PM -0400, Neil Leeder wrote: >>>> + /* Don't allow groups with mixed PMUs, except for s/w events */ >>>> + if (event->group_leader->pmu != event->pmu && >>>> + !is_software_event(event->group_leader)) { >>>> + dev_warn(&l2cache_pmu->pdev->dev, >>>> + "Can't create mixed PMU group\n"); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + list_for_each_entry(sibling, &event->group_leader->sibling_list, >>>> + group_entry) >>>> + if (sibling->pmu != event->pmu && >>>> + !is_software_event(sibling)) { >>>> + dev_warn(&l2cache_pmu->pdev->dev, >>>> + "Can't create mixed PMU group\n"); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + hwc->idx = -1; >>>> + hwc->config_base = event->attr.config; >>>> + >>>> + /* >>>> + * Ensure all events are on the same cpu so all events are in the >>>> + * same cpu context, to avoid races on pmu_enable etc. >>>> + */ >>>> + slice = get_hml2_pmu(event->cpu); >>>> + event->cpu = slice->on_cpu; >>> >>> This could put an event on a different CPU to its group siblings, which >>> is broken. >> >> This is the same logic as in arm-ccn.c:arm_ccn_pmu_event_init(), where there >> is a single CPU designated as the CPU to be used for all events. >> >> All events for this slice are forced to slice->on_cpu which is the CPU >> set in the cpumask for this slice. > > The CCN is a little different. For the CCN, a single CPU is designated > to handle *all* events. > > For this driver, a CPU is designated per-slice, judging by the existence > of hml2_pmu::on_cpu (unless that's superfluous). We've only verified > that the events are all for this PMU, not the same slice, and thus each > event->cpu may differ. > I see. So I can add a check that the group_leader event must be on the same slice, and thus on the same CPU. >> I'm not sure how this can put an event on a different CPU to its group >> siblings? > > In practice today, we'll try to schedule the event on it's group > leader's CPU, but accounting and subsequent manipulation could go wrong. > > Thanks, > Mark. > Neil -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.