On 07/09/2022 16:03, Dixit, Ashutosh wrote:
On Wed, 07 Sep 2022 00:28:48 -0700, Tvrtko Ursulin wrote:
On 06/09/2022 19:29, Umesh Nerlige Ramappa wrote:
On Thu, Sep 01, 2022 at 04:55:22PM -0700, Dixit, Ashutosh wrote:
On Wed, 31 Aug 2022 15:45:49 -0700, Umesh Nerlige Ramappa wrote:
[snip]
intel_gt_reset_unlock(gt, srcu);
@@ -1476,17 +1476,21 @@ void intel_guc_busyness_unpark(struct
intel_gt *gt)
guc->timestamp.ping_delay);
}
-static void __guc_context_update_clks(struct intel_context *ce)
+static u64 guc_context_update_stats(struct intel_context *ce)
{
struct intel_guc *guc = ce_to_guc(ce);
struct intel_gt *gt = ce->engine->gt;
u32 *pphwsp, last_switch, engine_id;
- u64 start_gt_clk, active;
unsigned long flags;
+ u64 total, active = 0;
ktime_t unused;
+ intel_context_pin(ce);
intel_context_pin can sleep and we are not allowed to sleep in this
path -
intel_context_get_total_runtime_ns(), however we can sleep in the ping
worker path, so ideally we want to separate it out for the 2 paths.
Do we know which intel_context_get_total_runtime_ns() call is not allowed
to sleep? E.g. the code path from i915_drm_client_fdinfo() is allowed to
sleep. As mentioned I have done this is v2 of RFC patch which I think is
sufficient, but a more complicated scheme (which I think we can avoid for
now) would be to pin in code paths when sleeping is allowed.
Hmm, maybe intel_context_get_total_runtime_ns can sleep, not sure why I
was assuming that this falls in the perf_pmu path. This is now in the
drm_fdinfo query path. + Tvrtko.
@Tvrtko, any idea if intel_context_get_total_runtime_ns path can sleep?
Not at the moment - it calls it from a lockless (rcu) section when walking
all the contexts belonging to a client. Idea being performance queries
should have minimum effect on a running system.
Hmm my bad, missing the rcu and assuming a userspace thread will be able to
sleep.
I think it would be possible to change in theory but not sure how much
work. There is a hard need to sleep in there or what?
GuC contexts need to be pinned/unpinned which can sleep but investigating
if we can return a previously computed busyness when we cannot pin/sleep.
Yeah it would be conceptually nice to keep that query light weight.
Doing too much work to query accumulated state, like in case of pinning
the unpinned contexts, feels a bit over the top. Hopefully there aren't
any nasty races which would make this hard.
Regards,
Tvrtko
