Re: [igt-dev] [PATCH i-g-t] tests/perf_pmu: Avoid off-lining all CPUs

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 19/02/2018 13:54, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-02-19 13:40:37)
> > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >
> > Bail out from the cpu-hotplug test if we failed to bring a CPU back
> > online.
> >
> > This still leaves the machine in a quite bad state, but at least it
> > avoids hard hanging it.
> >
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > ---
> >   tests/perf_pmu.c | 15 +++++++++++++--
> >   1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> > index 7fab73e22c2d..a334d3b5770e 100644
> > --- a/tests/perf_pmu.c
> > +++ b/tests/perf_pmu.c
> > @@ -965,6 +965,7 @@ static void cpu_hotplug(int gem_fd)
> >          int link[2];
> >          int fd, ret;
> >          int cur = 0;
> > +       char buf;
> >   
> >          igt_require(cpu0_hotplug_support());
> >   
> > @@ -1012,7 +1013,15 @@ static void cpu_hotplug(int gem_fd)
> >                          /* Offline followed by online a CPU. */
> >                          igt_assert_eq(write(cpufd, "0", 2), 2);
> >                          usleep(1e6);
> > -                       igt_assert_eq(write(cpufd, "1", 2), 2);
> > +                       ret = write(cpufd, "1", 2);
> > +                       if (ret < 0) {
> > +                               /*
> > +                                * Abort the test if we failed to bring a CPU
> > +                                * back online.
> > +                                */
> > +                               igt_assert_eq(write(link[1], "s", 1), 1);
> > +                               break;
> > +                       }
> >   
> >                          close(cpufd);
> >                          cpu++;
> > @@ -1026,7 +1035,6 @@ static void cpu_hotplug(int gem_fd)
> >           * until the CPU core shuffler finishes one loop.
> >           */
> >          for (;;) {
> > -               char buf;
> >                  int ret2;
> >   
> >                  usleep(500e3);
> > @@ -1053,6 +1061,9 @@ static void cpu_hotplug(int gem_fd)
> >          close(fd);
> >          close(link[0]);
> >   
> > +       /* Skip if child signals a problem with bringing a CPU back online. */
> > +       igt_skip_on(buf == 's');
>
> The logic makes sense.
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>

I wasn't so sure after I've sent it. There was an assert on write error 
already so that would have aborted the child. Will see what shards will 
say..

> What happens if we try to online an already on cpu? Will that report the
> failure or just tell us to stop wasting its time?

Seems the attempt is simply ignored.

Regards,

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