Re: [RFC 3/3] drm/i915: Export engine busy stats in debugfs

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

On 09/05/2017 19:17, Dmitry Rogozhkin wrote:
> On 5/9/2017 7:09 AM, Tvrtko Ursulin wrote:

[snip]

>> +static ssize_t i915_engine_stats_read(struct file *file, char __user
>> *ubuf,
>> +                      size_t count, loff_t *pos)
>> +{
>> +    struct i915_engine_stats_buf *buf =
>> +        (struct i915_engine_stats_buf *)file->private_data;
>> +
>> +    if (*pos == 0) {
>> +        struct drm_i915_private *dev_priv = file->f_inode->i_private;
>> +        char *ptr = &buf->buf[0];
>> +        int left = buf->len;
>> +        struct intel_engine_cs *engine;
>> +        enum intel_engine_id id;
>> +
>> +        buf->available = 0;
>> +
>> +        for_each_engine(engine, dev_priv, id) {
>> +            u64 total;
>> +            int len;
>> +
>> +            spin_lock_irq(&engine->stats.lock);
>> +            total = engine->stats.total;
>> +            /*
>> +             * If the engine is executing something at the moment
>> +             * add it to the total.
>> +             */
>> +            if (engine->stats.ref)
>> +                total += ktime_get_real_ns() -
>> +                     engine->stats.start;
>> +            spin_unlock_irq(&engine->stats.lock);
>> +
>> +            len = snprintf(ptr, left, "%s: %llu\n",
>> +                       engine->name, total);
> If I caught it right, file format is:
>   render ring: 12345
>   bsd ring: 12345
>   ...

Yes almost, just that the engine names have been changed to likes of 
rcs0, vcs0, vcs1, vecs0 and bcs0 in the meantime.

> where numbers are busy clocks (ns) from the system boot time. Is that

Nanoseconds, but not since boot time but since the last time tracking 
got enabled.

Because the most important thing in this version, from the point of view 
of overhead in interrupt tasklet, is that the tracking is not done 
unless somebody is listening (has the file open).

As I wrote in the cover letter and the 2nd patch, when nobody has the 
file open the only thing which exists in the interrupt tasklets are 
three no-nop instructions. They only get patched to jumps (to sections 
actually collecting the stats) for as long as someone has the file open.

> right? What if we will want to expose some other statistics information
> later, not only busy clocks? For example, engines i915 queues depths is
> a next interest. Maybe later we will find something else interesting.
> So, do we want to consider this file to contain all kind of statistics
> in the future, and hence it should be of somewhat different format, or
> it will have only busy clocks, and maybe we need other file name then?

It can be either of the two, or some third option. It sounds like it is 
too early to discuss those level of detail. At this point it was an RFC 
only to gather some opinions on the overall idea.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx