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

[Andi Shyti]

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

Andi

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