Re: [Intel-gfx] [RFC PATCH 2/2] Fix per client busyness locking

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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. 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?

Regards,

Tvrtko