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;
Date:   Wed, 14 Aug 2019 16:50:58 -0700
In-Reply-To: <CANLsYkz3_bzRCQEVb00Tbf3Rdww13mePN-woncctOu7OanF00A@mail.gmail.com>
Message-Id: <20190814235058.184204-1-yabinc@google.com>
Mime-Version: 1.0
References: <CANLsYkz3_bzRCQEVb00Tbf3Rdww13mePN-woncctOu7OanF00A@mail.gmail.com>
Subject: Re: [PATCH v2] coresight: tmc-etr: Fix perf_data check.
From:   Yabin Cui <yabinc@google.com>
To:     Mathieu Poirier <mathieu.poirier@linaro.org>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Yabin Cui <yabinc@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> Did you actually see the check fail or is this a theoretical thing?
> I'm really perplex here has I have tested this scenario many times
> without issues.
>
I have seen this warning in dmesg output, that's how I find the problem.

> In CPU wide scenarios each perf event (one per CPU) is associated with
> an event_data during the setup process.  The event_data is the
> etr_perf holding a reference to the perf ring buffer for that specific
> event along with the etr_buf, regardless of who created the latter.

Agree.

> From there, when the event is installed on a CPU, the csdev for that
> CPU is given a reference to the event_data of that event[1].  Before
> going further notice how there is a per CPU csdev and event handle to
> keep track of event specifics[2]. As such both (per CPU) csdev and
> event handle carry the exact same reference to the etr_perf.
>
On my test device (Pixel 3), there is an ETM device on each cpu, but only
one ETR device for the whole device. So there is only one instance of etr
csdev in the kernel. If multiple cpus are scheduling on etm perf events at
the same time, all of them are trying to set their event_data to the same
etr csdev. And different perf events have different event_data. A warning
situation is as below:

   cpu 0
   schedule on event A (set etr csdev->perf_data to event_a.etr_perf)

   cpu 1
   schedule on event B (set etr csdev->perf_data to event_b.etr_perf)

   cpu 1
   schedule off event B (update buffer, does nothing since csdev->refcnt != 1)

   cpu 0
   schedule off event A (update buffer, but etr csdev->perf_data check fail)