[Intel-xe] [PATCH 1/2] drm/xe: add a new sysfs directory for gtidle properties

[Riana Tauro]

On 6/15/2023 3:19 PM, Andi Shyti wrote:
> Hi Riana,
> 
>>>> +static u64 get_residency_ms(struct xe_gt_idle *gtidle, u64 cur_residency)
>>>> +{
>>>> +	u64 delta, overflow_residency, prev_residency;
>>>> +
>>>> +	overflow_residency = BIT_ULL(32);
>>>> +
>>>> +	/*
>>>> +	 * Counter wrap handling
>>>> +	 * Store previous hw counter values for counter wrap-around handling
>>>> +	 * Relying on sufficient frequency of queries otherwise counters can still wrap.
>>>> +	 */
>>>> +	prev_residency = gtidle->prev_residency;
>>>> +	gtidle->prev_residency = cur_residency;
>>>> +
>>>> +	/* delta */
>>>> +	if (cur_residency >= prev_residency)
>>>> +		delta = cur_residency - prev_residency;
>>>> +	else
>>>> +		delta = cur_residency + (overflow_residency - prev_residency);
>>>> +
>>>> +	/* Add delta to extended raw driver copy of idle residency */
>>>> +	cur_residency = gtidle->cur_residency + delta;
>>>> +	gtidle->cur_residency = cur_residency;
>>>> +
>>>> +	/* residency multiplier in ns, convert to ms */
>>>> +	cur_residency = mul_u64_u32_div(cur_residency, gtidle->residency_multiplier, 1e6);
>>>
>>> do we need to lock something here? Looks a bit racy to me.
> 
>> seq_read_iter takes a lock while calling sysfs_show
>> https://elixir.bootlin.com/linux/latest/source/fs/seq_file.c#L171
>>
>> This function is used only by idle_residency_show. So didn't add the lock
> 
> I was just thinking a lock around gtidle. the m->lock from
> seq_read_iter is loking on the seq_file m so that consecutive
> reads/writes cannot be made.
> 
> If gtidle is not used anywhere else, then I guess locking is
> safe and for now "struct xe_gt_idle" is in sysfs (but still in
> the xe_gt).

Hi Andi

All gt idle properties are used only in sysfs. If it gets used anywhere 
else will add a lock

Thanks
Riana

> 
> Andi
> 
>>>> +	return cur_residency;
>>>> +}