Quoting Tvrtko Ursulin (2021-01-29 09:18:50)
>
>
> On 26/01/2021 13:05, Chris Wilson wrote:
> > The client id used is a cyclic allocator as that reduces the likelihood
> > of userspace seeing the same id used again (and so confusing the new
> > client as the old). Verify that each new client has an id greater than
> > the last.
> >
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
> > ---
> > tests/i915/sysfs_clients.c | 129 +++++++++++++++++++++++++++++++------
> > 1 file changed, 108 insertions(+), 21 deletions(-)
> >
> > diff --git a/tests/i915/sysfs_clients.c b/tests/i915/sysfs_clients.c
> > index a3a1f81e1..d2c1ebc5f 100644
> > --- a/tests/i915/sysfs_clients.c
> > +++ b/tests/i915/sysfs_clients.c
> > @@ -8,6 +8,7 @@
> > #include <errno.h>
> > #include <fcntl.h>
> > #include <inttypes.h>
> > +#include <limits.h>
> > #include <math.h>
> > #include <sched.h>
> > #include <sys/socket.h>
> > @@ -47,6 +48,8 @@
> > #define assert_within_epsilon(x, ref, tolerance) \
> > __assert_within_epsilon(x, ref, tolerance / 100., tolerance / 100.)
> >
> > +#define BUFSZ 280
> > +
> > #define MI_BATCH_BUFFER_START (0x31 << 23)
> > #define MI_BATCH_BUFFER_END (0xa << 23)
> > #define MI_ARB_CHECK (0x5 << 23)
> > @@ -75,7 +78,7 @@ static void pidname(int i915, int clients)
> > {
> > struct dirent *de;
> > int sv[2], rv[2];
> > - char buf[280];
> > + char buf[BUFSZ];
> > int me = -1;
> > long count;
> > pid_t pid;
> > @@ -180,7 +183,7 @@ static long count_clients(int clients)
> > {
> > struct dirent *de;
> > long count = 0;
> > - char buf[280];
> > + char buf[BUFSZ];
> > DIR *dir;
> >
> > dir = fdopendir(dup(clients));
> > @@ -229,32 +232,113 @@ static void create(int i915, int clients)
> > igt_assert_eq(count_clients(clients), 1);
> > }
> >
> > +static const char *find_client(int clients, pid_t pid, char *buf)
> > +{
> > + DIR *dir = fdopendir(dup(clients));
> > +
> > + /* Reading a dir as it changes does not appear to be stable, SEP */
> > + for (int pass = 0; pass < 2; pass++) {
> > + struct dirent *de;
> > +
> > + rewinddir(dir);
> > + while ((de = readdir(dir))) {
> > + if (!isdigit(de->d_name[0]))
> > + continue;
> > +
> > + snprintf(buf, BUFSZ, "%s/pid", de->d_name);
> > + igt_sysfs_read(clients, buf, buf, sizeof(buf));
> > + if (atoi(buf) != pid)
> > + continue;
> > +
> > + strncpy(buf, de->d_name, BUFSZ);
> > + goto out;
> > + }
> > + }
> > + *buf = '\0';
> > +out:
> > + closedir(dir);
> > + return buf;
> > +}
> > +
> > static int find_me(int clients, pid_t pid)
> > {
> > - struct dirent *de;
> > - char buf[280];
> > - int me = -1;
> > - DIR *dir;
> > + char buf[BUFSZ];
> >
> > - dir = fdopendir(dup(clients));
> > - igt_assert(dir);
> > - rewinddir(dir);
> > + return openat(clients,
> > + find_client(clients, pid, buf),
> > + O_DIRECTORY | O_RDONLY);
> > +}
> >
> > - while ((de = readdir(dir))) {
> > - if (!isdigit(de->d_name[0]))
> > - continue;
> > +static int reopen_directory(int fd)
> > +{
> > + char buf[BUFSZ];
> > + int dir;
> >
> > - snprintf(buf, sizeof(buf), "%s/pid", de->d_name);
> > - igt_sysfs_read(clients, buf, buf, sizeof(buf));
> > - if (atoi(buf) != pid)
> > - continue;
> > + snprintf(buf, sizeof(buf), "/proc/self/fd/%d", fd);
> > + dir = open(buf, O_RDONLY);
>
> Maybe O_DIRECTORY if it is open_directory.
>
> > + igt_assert_fd(dir);
> >
> > - me = openat(clients, de->d_name, O_DIRECTORY | O_RDONLY);
> > - break;
> > + return dir;
> > +}
> > +
> > +static unsigned int my_id(int clients, pid_t pid)
> > +{
> > + char buf[BUFSZ];
> > +
> > + return atoi(find_client(clients, pid, buf));
> > +}
> > +
> > +static unsigned int recycle_client(int i915, int clients)
> > +{
> > + int device, client;
> > +
> > + device = gem_reopen_driver(i915);
> > + client = my_id(clients, getpid());
> > + close(device);
> > +
> > + return client;
> > +}
> > +
> > +static void recycle(int i915, int clients)
> > +{
> > + const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> > +
> > + /*
> > + * As we open and close clients, we do not expect to reuse old ids,
> > + * i.e. we use a cyclic ida. This reduces the likelihood of userspace
> > + * watchers becoming confused and mistaking the new client as a
> > + * continuation of the old.
> > + */
> > + igt_require(my_id(clients, getpid()) < INT_MAX / 2);
>
> Hm this is a bit dodgy - it will cause "permanent" skips if running the
> test in a loop. Just for the client > last assert below? I guess it is
> hard to handle wrap with forked clients.
It takes about a day to reach 2 billion ids on a fast machine. For CI, I
think we are safe.
We could do the (int)(A - B) > 0 to handle the wrap, that would be more
sensible.
[...]
> Okay better than nothing.
But first we need to resolve the failure to find itself. :(
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
