Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: BOf9CFe584nXIWoc6rLLnGG6wtbu1UH7wvvJZE662lmmJWXd7iRS3L5UvhUprTcY3xPz+4swup
 ZSB/2LEWHTow==
IronPort-SDR: tmWePrX5hy4Kaid55JWDoJLvTsGoycmX2AjTKagX4RBRNm0XSGKcOL7qnW6TPf0e+lDwc6v65z
 3xNQ50q02UlQ==
Subject: Re: [PATCH V6 09/14] perf/x86/intel: Support TopDown metrics on Ice
 Lake
To:     peterz@infradead.org
Cc:     acme@redhat.com, mingo@kernel.org, linux-kernel@vger.kernel.org,
        jolsa@kernel.org, eranian@google.com,
        alexander.shishkin@linux.intel.com, ak@linux.intel.com
References: <20200717140554.22863-1-kan.liang@linux.intel.com>
 <20200717140554.22863-10-kan.liang@linux.intel.com>
 <20200721124042.GX10769@hirez.programming.kicks-ass.net>
 <e9f855ad-e4bd-8766-29b6-d251c859d58f@linux.intel.com>
 <20200721143110.GL119549@hirez.programming.kicks-ass.net>
From:   "Liang, Kan" <kan.liang@linux.intel.com>
Message-ID: <59795cbd-a95c-8fdd-522d-199fbb047b8a@linux.intel.com>
Date:   Tue, 21 Jul 2020 11:50:49 -0400
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20200721143110.GL119549@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 7/21/2020 10:31 AM, peterz@infradead.org wrote:
> On Tue, Jul 21, 2020 at 10:23:36AM -0400, Liang, Kan wrote:
> 
>> Patch 13 forces the slots event to be part of a metric group. In patch 7,
>> for a metric group, we only update the values once with slots event.
>> I think the normal case mentioned above should not happen.
>>
>> +	/* Only need to call update_topdown_event() once for group read. */
>> +	if ((cpuc->txn_flags & PERF_PMU_TXN_READ) &&
>> +	    !is_slots_event(event))
>> +		return;
>> +
>> +	perf_pmu_disable(event->pmu);
>> +	x86_pmu.update_topdown_event(event);
>> +	perf_pmu_enable(event->pmu);
> 
> Ah, I missed that.
> 
> That also requires SLOTS to be the leader so that it will be the first
> read. Did we enforce that somewhere?

I think it's missed in the current patch set. The suggested code in 
patch 11 forces SLOTS to be the leader. I will apply it in V7.

Thanks,
Kan