[PATCH i-g-t v8 3/8] lib/drmtest: allow opening cards out of order
Zbigniew Kempczyński
zbigniew.kempczynski at intel.com
Fri Feb 9 12:38:20 UTC 2024
On Thu, Feb 08, 2024 at 02:52:27PM +0100, 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 and add some more debug prints.
>
> 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
> v7: restore check for already opened filtered card (Janusz)
> v8: limit checks for N-th card to 0...N-1 range (Janusz)
> simplify if-else in __search_and_open() (Kamil)
>
> 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 | 73 +++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 56 insertions(+), 17 deletions(-)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 73d9159af..62d31b000 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;
Looks _opened_fds_count is monotonic over test runtime. I mean if you're
running single test which is multigpu which just exists it doesn't
matter, but what will happen if you'll run multigpu subtest after which
you have test with single fd usage? I mean _opened_fd[] has stale data.
> }
>
> -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(as_idx, _opened_fds_count); 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,37 @@ 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)
> + continue;
> +
> + ++idx;
> + if (idx == as_idx)
> return fd;
Shouldn't we mark this fd is already opened in _opened_fds[]?
--
Zbigniew
> +
> + close(fd);
> }
>
> return -1;
> @@ -486,7 +519,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
> + * * if it's already opened (for indexes != idx) we fail with -1
> * * otherwise open the device and return the fd
> *
> * 2. compatibility mode - open the first DRM device we can find that is not
> @@ -531,6 +564,7 @@ int __drm_open_driver_another(int idx, int chipset)
> if (chipset != DRIVER_VGEM && igt_device_filter_count() > idx) {
> struct igt_device_card card;
> bool found;
> + int err;
>
> found = __get_card_for_nth_filter(idx, &card);
>
> @@ -539,14 +573,19 @@ int __drm_open_driver_another(int idx, int chipset)
> found = __get_card_for_nth_filter(idx, &card);
> }
>
> - if (!found || !strlen(card.card))
> + 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 if (_is_already_opened(card.card, idx, &err)) {
> + if (err < 0)
> + igt_warn("cannot stat card maching filter %d errno=%d card=%s\n", idx, err, card.card);
> + else if (idx != err)
> + igt_warn("card maching filter %d is already opened as %d, card=%s\n", idx, err, card.card);
> + else
> + fd = __open_driver_exact(card.card, chipset);
> + } else {
> 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);
> --
> 2.42.0
>
More information about the igt-dev
mailing list