Re: [PATCH i-g-t 6/6] tests/gem_exec_latency: New subtests for checking submission from RT tasks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Tvrtko Ursulin (2018-05-14 12:13:10)
> 
> On 14/05/2018 09:02, Chris Wilson wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > 
> > We want to make sure RT tasks which use a lot of CPU times can submit
> > batch buffers with roughly the same latency (and certainly not worse)
> > compared to normal tasks.
> > 
> > v2: Add tests to run across all engines simultaneously to encourage
> > ksoftirqd to kick in even more often.
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> #v1
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> >   tests/gem_exec_latency.c | 207 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 207 insertions(+)
> > 
> > diff --git a/tests/gem_exec_latency.c b/tests/gem_exec_latency.c
> > index 9498c0921..c5816427b 100644
> > --- a/tests/gem_exec_latency.c
> > +++ b/tests/gem_exec_latency.c
> > @@ -36,11 +36,15 @@
> >   #include <sys/time.h>
> >   #include <sys/signal.h>
> >   #include <time.h>
> > +#include <sched.h>
> >   
> >   #include "drm.h"
> >   
> >   #include "igt_sysfs.h"
> >   #include "igt_vgem.h"
> > +#include "igt_dummyload.h"
> > +#include "igt_stats.h"
> > +
> >   #include "i915/gem_ring.h"
> >   
> >   #define LOCAL_I915_EXEC_NO_RELOC (1<<11)
> > @@ -351,6 +355,189 @@ static void latency_from_ring(int fd,
> >       }
> >   }
> >   
> > +static void __rearm_spin_batch(igt_spin_t *spin)
> > +{
> > +     const uint32_t mi_arb_chk = 0x5 << 23;
> > +
> > +       *spin->batch = mi_arb_chk;
> > +       *spin->running = 0;
> > +       __sync_synchronize();
> > +}
> > +
> > +static void
> > +__submit_spin_batch(int fd, igt_spin_t *spin, unsigned int flags)
> > +{
> > +     struct drm_i915_gem_execbuffer2 eb = spin->execbuf;
> > +
> > +     eb.flags &= ~(0x3f | I915_EXEC_BSD_MASK);
> > +     eb.flags |= flags | I915_EXEC_NO_RELOC;
> > +
> > +     gem_execbuf(fd, &eb);
> > +}
> > +
> > +struct rt_pkt {
> > +     struct igt_mean mean;
> > +     double min, max;
> > +};
> > +
> > +static bool __spin_wait(int fd, igt_spin_t *spin)
> > +{
> > +     while (!READ_ONCE(*spin->running)) {
> > +             if (!gem_bo_busy(fd, spin->handle))
> > +                     return false;
> > +     }
> > +
> > +     return true;
> > +}
> > +
> > +/*
> > + * Test whether RT thread which hogs the CPU a lot can submit work with
> > + * reasonable latency.
> > + */
> > +static void
> > +rthog_latency_on_ring(int fd, unsigned int engine, const char *name, unsigned int flags)
> > +#define RTIDLE 0x1
> > +{
> > +     const char *passname[] = { "warmup", "normal", "rt" };
> > +     struct rt_pkt *results;
> > +     unsigned int engines[16];
> > +     const char *names[16];
> > +     unsigned int nengine;
> > +     int ret;
> > +
> > +     results = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
> > +     igt_assert(results != MAP_FAILED);
> > +
> > +     nengine = 0;
> > +     if (engine == ALL_ENGINES) {
> > +             for_each_physical_engine(fd, engine) {
> > +                     if (!gem_can_store_dword(fd, engine))
> > +                             continue;
> > +
> > +                     engines[nengine] = engine;
> > +                     names[nengine] = e__->name;
> > +                     nengine++;
> > +             }
> > +             igt_require(nengine > 1);
> > +     } else {
> > +             igt_require(gem_can_store_dword(fd, engine));
> > +             engines[nengine] = engine;
> > +             names[nengine] = name;
> > +             nengine++;
> > +     }
> > +
> > +     igt_fork(child, nengine) {
> > +             unsigned int pass = 0; /* Three passes: warmup, normal, rt. */
> > +
> > +             engine = engines[child];
> > +             do {
> > +                     struct igt_mean mean;
> > +                     double min = HUGE_VAL;
> > +                     double max = -HUGE_VAL;
> > +                     igt_spin_t *spin;
> > +
> > +                     igt_mean_init(&mean);
> > +
> > +                     if (pass == 2) {
> > +                             struct sched_param rt =
> > +                             { .sched_priority = 99 };
> > +
> > +                             ret = sched_setscheduler(0,
> > +                                                      SCHED_FIFO | SCHED_RESET_ON_FORK,
> > +                                                      &rt);
> > +                             if (ret) {
> > +                                     igt_warn("Failed to set scheduling policy!\n");
> > +                                     break;
> > +                             }
> > +                     }
> > +
> > +                     spin = __igt_spin_batch_new_poll(fd, 0, engine);
> > +                     if (!spin) {
> > +                             igt_warn("Failed to create spinner! (%s)\n",
> > +                                      passname[pass]);
> > +                             break;
> > +                     }
> > +                     igt_spin_busywait_until_running(spin);
> > +
> > +                     igt_until_timeout(pass > 0 ? 5 : 2) {
> > +                             struct timespec ts = { };
> > +                             double t;
> > +
> > +                             igt_spin_batch_end(spin);
> > +                             gem_sync(fd, spin->handle);
> > +                             if (flags & RTIDLE)
> > +                                     usleep(250);
> 
> 250us is how long you expect context complete to be received in?
> 
> s/RTIDLE/SUBMIT_IDLE/ ?

And the others. I guess DROP_IDLE (i.e. do a wait-for-idle might be
fun).

> > +
> > +                             /*
> > +                              * If we are oversubscribed (more RT hogs than
> > +                              * cpus) give the others a change to run;
> > +                              * otherwise, they will interrupt us in the
> > +                              * middle of the measurement.
> > +                              */
> > +                             if (nengine > 1)
> > +                                     usleep(10*nengine);
> 
> Could check for actual number of cpus here.

Could be that fancy. There's still the issue that if we consume
more than 70% (or whatever the rt limit is) we can be throttled. So we
do need some sort of scheduler relinquishment.
 
> You want it on top of the RTIDLE sleep?

It's peanuts on top, so I don't mind.
> 
> > +
> > +                             __rearm_spin_batch(spin);
> > +
> > +                             igt_nsec_elapsed(&ts);
> > +                             __submit_spin_batch(fd, spin, engine);
> > +                             if (!__spin_wait(fd, spin)) {
> > +                                     igt_warn("Wait timeout! (%s)\n",
> > +                                              passname[pass]);
> 
> It's not really a timeout how __spin_wait is implemented.

Can't blame me for this one ;)

> > +                                     break;
> > +                             }
> > +
> > +                             t = igt_nsec_elapsed(&ts) * 1e-9;
> > +                             if (t > max)
> > +                                     max = t;
> > +                             if (t < min)
> > +                                     min = t;
> > +
> > +                             igt_mean_add(&mean, t);
> > +                     }
> > +
> > +                     igt_spin_batch_free(fd, spin);
> > +
> > +                     igt_info("%8s %10s: mean=%.2fus stddev=%.3fus [%.2fus, %.2fus] (n=%lu)\n",
> > +                              names[child],
> > +                              passname[pass],
> > +                              igt_mean_get(&mean) * 1e6,
> > +                              sqrt(igt_mean_get_variance(&mean)) * 1e6, > +                           min * 1e6, max * 1e6,
> > +                              mean.count);
> > +
> > +                     results[3 * child + pass].mean = mean;
> > +                     results[3 * child + pass].min = min;
> > +                     results[3 * child + pass].max = max;
> > +             } while (++pass < 3);
> > +     }
> > +
> > +     igt_waitchildren();
> > +
> > +     for (unsigned int child = 0; child < nengine; child++) {
> > +             struct rt_pkt normal = results[3 * child + 1];
> > +             struct rt_pkt rt = results[3 * child + 2];
> > +
> > +             igt_assert(rt.max);
> > +
> > +             igt_info("%8s: normal latency=%.2f±%.3fus, rt latency=%.2f±%.3fus\n",
> > +                      names[child],
> > +                      igt_mean_get(&normal.mean) * 1e6,
> > +                      sqrt(igt_mean_get_variance(&normal.mean)) * 1e6,
> > +                      igt_mean_get(&rt.mean) * 1e6,
> > +                      sqrt(igt_mean_get_variance(&rt.mean)) * 1e6);
> > +
> > +             igt_assert(igt_mean_get(&rt.mean) <
> > +                        igt_mean_get(&normal.mean) * 2);
> > +
> > +             /* The system is noisy; be conservative when declaring fail. */
> > +             igt_assert(igt_mean_get_variance(&rt.mean) <
> > +                        igt_mean_get_variance(&normal.mean) * 25);
> 
> I don't know what "* 25" means in statistical terms. At some point you 
> said variance doesn't work and you had to go with stdev?

5 sigma. I was complaining that reporting variance as 0.000us wasn't
particularly useful (wrong units for starters ;) and that we definitely
could meet the 3*variance requirement :)

At this moment, I'm plucking numbers to try and safely fail when it's
bad (we hit ksoftird and it's order of magnitude worse), but enough
slack for noise. The mean so far looks more reliable than the
variance/stddev, but testing the variability makes sense. Oh well, if
the system is that unstable, we just could use more passes to improve the
statistics.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux