[PATCH i-g-t v6 3/8] lib/drmtest: allow opening cards out of order
Kamil Konieczny
kamil.konieczny at linux.intel.com
Fri Feb 2 15:32:32 UTC 2024
Hi,
On 2024-02-02 at 12:38:21 +0100, Janusz Krzysztofik wrote:
> On Thursday, 1 February 2024 16:50:42 CET Kamil Konieczny wrote:
> > Current __drm_open_driver_another() implementation prevents
> > opening cards out of order, for example card 3 without previous
> > opens of 0..2. This can be seen with three GPU cards system and
> > gem_open_basic test:
> >
> > sudo IGT_DEVICE=pci:vendor=Intel,card=all build/tests/gem_basic --r multigpu-create-close
> >
> > IGT-Version: 1.28-g2548a539e (x86_64) (Linux: 6.7.0-rc5 x86_64)
> > Using IGT_SRANDOM=1706630533 for randomisation
> > Opened device: /dev/dri/card0
> > Starting subtest: multigpu-create-close
> > <g:0> Testing creating and closing an object.
> > <g:1> Opened device: /dev/dri/card1
> > <g:1> Testing creating and closing an object.
> > gem_basic: ../lib/drmtest.c:341: _is_already_opened: Assertion `as_idx <= _opened_fds_count' failed.
> > Received signal SIGABRT.
> >
> > Relax that condition. Also relax condition for filtered devices and
>
> Can we have those two parts split into two patches, please? If not then could
> you please explain why (how they depend on each other)?
I will remove this and you can see how it looks with old
conditions checked, e.g. when n-th card should be different
from m-th card, for n != m. I can reintroduce that second
behaviour later.
Regards,
Kamil
>
> Thanks,
> Janusz
>
>
> > allow them to open already opened ones, basically it will allow
> > running multi-GPU scenarios on single-GPU system with filter:
> >
> > IGT_DEVICE=pci:vendor=Intel,card=0\;pci:vendor=Intel,card=0
> >
> > Changes in __search_and_open() was suggested by Zbigniew.
> >
> > v5:
> > removed caching for names of opened cards
> > relaxed opening of filtered devices
> > v6:
> > fixed assingment in _is_already_open() (Janusz)
> > simplify use of this function
> >
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Cc: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> > Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > ---
> > lib/drmtest.c | 66 ++++++++++++++++++++++++++++++++++++++-------------
> > 1 file changed, 49 insertions(+), 17 deletions(-)
> >
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index 73d9159af..5bb084508 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -322,35 +322,54 @@ static int _opened_fds_count;
> > static void _set_opened_fd(int idx, int fd)
> > {
> > assert(idx < ARRAY_SIZE(_opened_fds));
> > - assert(idx <= _opened_fds_count);
> > + assert(idx >= 0);
> >
> > _opened_fds[idx].fd = fd;
> >
> > assert(fstat(fd, &_opened_fds[idx].stat) == 0);
> >
> > - _opened_fds_count = idx+1;
> > + for (int n = _opened_fds_count; n < idx; n++)
> > + _opened_fds[n].fd = -1;
> > +
> > + if (idx >= _opened_fds_count)
> > + _opened_fds_count = idx + 1;
> > }
> >
> > -static bool _is_already_opened(const char *path, int as_idx)
> > +/* in *err returns -errno or index to _opened_fds[] */
> > +static bool _is_already_opened(const char *path, int as_idx, int *err)
> > {
> > struct stat new;
> >
> > assert(as_idx < ARRAY_SIZE(_opened_fds));
> > - assert(as_idx <= _opened_fds_count);
> > + assert(as_idx >= 0);
> >
> > /*
> > * we cannot even stat the device, so it's of no use - let's claim it's
> > * already opened
> > */
> > - if (igt_debug_on(stat(path, &new) != 0))
> > + if (igt_debug_on(stat(path, &new) != 0)) {
> > + if (err)
> > + *err = -errno;
> > +
> > return true;
> > + }
> > +
> > + if (err)
> > + *err = 0;
> > +
> > + for (int i = 0, end = min(_opened_fds_count, as_idx); i < end; ++i) {
> > + if (_opened_fds[i].fd == -1)
> > + continue;
> >
> > - for (int i = 0; i < as_idx; ++i) {
> > /* did we cross filesystem boundary? */
> > assert(_opened_fds[i].stat.st_dev == new.st_dev);
> >
> > - if (_opened_fds[i].stat.st_ino == new.st_ino)
> > + if (_opened_fds[i].stat.st_ino == new.st_ino) {
> > + if (err)
> > + *err = i;
> > +
> > return true;
> > + }
> > }
> >
> > return false;
> > @@ -359,23 +378,38 @@ static bool _is_already_opened(const char *path, int as_idx)
> > static int __search_and_open(const char *base, int offset, unsigned int chipset, int as_idx)
> > {
> > const char *forced;
> > + int err;
> >
> > forced = forced_driver();
> > if (forced)
> > igt_debug("Force option used: Using driver %s\n", forced);
> >
> > - for (int i = 0; i < 16; i++) {
> > + for (int i = 0, idx = -1; i < 16 && idx < as_idx; i++) {
> > char name[80];
> > int fd;
> >
> > sprintf(name, "%s%u", base, i + offset);
> >
> > - if (_is_already_opened(name, as_idx))
> > - continue;
> > + if (_is_already_opened(name, as_idx, &err)) {
> > + if (err < 0)
> > + continue;
> > +
> > + if (idx + 1 < as_idx) {
> > + ++idx;
> > + continue;
> > + }
> > + }
> >
> > fd = __drm_open_device(name, chipset);
> > - if (fd != -1)
> > + if (fd != -1) {
> > + ++idx;
> > + if (idx < as_idx) {
> > + close(fd);
> > + continue;
> > + }
> > +
> > return fd;
> > + }
> > }
> >
> > return -1;
> > @@ -486,8 +520,7 @@ static bool __get_card_for_nth_filter(int idx, struct igt_device_card *card)
> > * * idx-th filter (starting with 0, filters are semicolon separated) is used
> > * * if there is no idx-th filter, goto 2
> > * * first device maching the filter is selected
> > - * * if it's already opened (for indexes = 0..idx-1) we fail with -1
> > - * * otherwise open the device and return the fd
> > + * * open the device and return the fd
> > *
> > * 2. compatibility mode - open the first DRM device we can find that is not
> > * already opened for indexes 0..idx-1, searching up to 16 device nodes
> > @@ -542,11 +575,10 @@ int __drm_open_driver_another(int idx, int chipset)
> > if (!found || !strlen(card.card))
> > igt_warn("No card matches the filter! [%s]\n",
> > igt_device_filter_get(idx));
> > - else if (_is_already_opened(card.card, idx))
> > - igt_warn("card maching filter %d is already opened\n", idx);
> > - else
> > + else {
> > + _is_already_opened(card.card, idx, NULL); /* check only filesystem boundary */
> > fd = __open_driver_exact(card.card, chipset);
> > -
> > + }
> > } else {
> > /* no filter for device idx, let's open whatever is available */
> > fd = __open_driver("/dev/dri/card", 0, chipset, idx);
> >
>
>
>
>
More information about the igt-dev
mailing list