* [PATCH i-g-t v2 v2] tests/core_setmaster: simplify device handling
@ 2024-10-04 15:01 Bommu Krishnaiah
2024-10-04 15:42 ` Ghimiray, Himal Prasad
0 siblings, 1 reply; 2+ messages in thread
From: Bommu Krishnaiah @ 2024-10-04 15:01 UTC (permalink / raw)
To: igt-dev
Cc: Bommu Krishnaiah, Emil Velikov, Himal Prasad Ghimiray,
Kamil Konieczny
Refactored test setup to dynamically determine the correct drm card
for testing. Instead of changing permissions for all cards,
change only one that will be tested.
Problem Description:
The test fails after running the `core_hotunplug@hot*` subtest, where
Card0 is populated as card1, This leads to a failure in the subsequent
`master-drop-set-user` test.
Command sequence for reproducing the issue:
- Check available cards: `ls /dev/dri/`
- Output: by-path card0 renderD128
- Run the test: `# ./core_hotunplug --r hotrebind-lateclose`
- Check again: ‘ls /dev/dri/’
- Output after core_hotunplug : by-path card1 renderD129
- Run the test: `# ./core_setmaster --r master-drop-set-user`
- Output: gta@core_setmaster@master-drop-set-user failure
This failures on both XE and i915 for above sequence.
v2: Currented the e-mail address
Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@intel.com>
---
tests/core_setmaster.c | 82 ++++++++++++++++++++++++------------------
1 file changed, 47 insertions(+), 35 deletions(-)
diff --git a/tests/core_setmaster.c b/tests/core_setmaster.c
index 9c2083f66..fe9993515 100644
--- a/tests/core_setmaster.c
+++ b/tests/core_setmaster.c
@@ -107,40 +107,31 @@ static void check_drop_set(void)
drm_close_driver(master);
}
-static unsigned tweak_perm(uint8_t *saved_perm, unsigned max_perm, bool save)
+static void tweak_perm(uint8_t *saved_perm, char *path, bool save)
{
- char path[256];
struct stat st;
- unsigned i;
-
- for (i = 0; i < max_perm; i++) {
- snprintf(path, sizeof(path), "/dev/dri/card%u", i);
-
- /* Existing userspace assumes there's no gaps, do the same. */
- if (stat(path, &st) != 0)
- break;
-
- if (save) {
- /* Save and toggle */
- saved_perm[i] = st.st_mode & (S_IROTH | S_IWOTH);
- st.st_mode |= S_IROTH | S_IWOTH;
- } else {
- /* Clear and restore */
- st.st_mode &= ~(S_IROTH | S_IWOTH);
- st.st_mode |= saved_perm[i];
- }
-
- /* There's only one way for chmod to fail - race vs rmmod.
- * In that case, do _not_ error/skip, since:
- * - we need to restore the [correct] permissions
- * - __drm_open_driver() can open another device, aka the
- * failure may be irrelevant.
- */
- chmod(path, st.st_mode);
+ int ret;
+
+ if (path[0] == 0)
+ return;
+
+ ret = stat(path, &st);
+ igt_assert_f(!ret, "stat failed with %d path=%s\n", errno, path);
+
+ if (save) {
+ /* Save and toggle */
+ *saved_perm = st.st_mode & (S_IROTH | S_IWOTH);
+ st.st_mode |= S_IROTH | S_IWOTH;
+ } else {
+ /* Clear and restore */
+ st.st_mode &= ~(S_IROTH | S_IWOTH);
+ st.st_mode |= *saved_perm;
}
- return i;
-}
+ /* There's only one way for chmod to fail - race vs rmmod. */
+ ret = chmod(path, st.st_mode);
+ igt_assert_f(!ret, "chmod failed with %d path=%s\n", errno, path);
+}
igt_main
{
@@ -160,8 +151,8 @@ igt_main
igt_subtest_group {
- uint8_t saved_perm[255];
- unsigned num;
+ uint8_t saved_perm;
+ char buf[255];
/* Upon dropping root we end up as random user, which
* a) is not in the video group, and
@@ -176,8 +167,29 @@ igt_main
* restored on skip or failure.
*/
igt_fixture {
- num = tweak_perm(saved_perm, ARRAY_SIZE(saved_perm),
- true);
+ char path[255];
+ int len;
+ int fd;
+
+ memset(buf, 0, sizeof(buf));
+ saved_perm = 0;
+
+ fd = __drm_open_driver(DRIVER_ANY);
+ igt_assert_fd(fd);
+
+ snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
+
+ len = readlink(path, buf, sizeof(buf) - 1);
+ igt_assert_f(len > 0, "readlink failed with %d path=%s\n", errno, path);
+
+ buf[len] = '\0';
+ if (!(strstr(buf, "/dev/dri/card") == buf ||
+ strstr(buf, "/dev/dri/renderD") == buf))
+ igt_assert_f(0, "Not a card nor render, path=%s\n", buf);
+
+ igt_assert_eq(__drm_close_driver(fd), 0);
+
+ tweak_perm(&saved_perm, buf, true);
}
igt_describe("Ensure first normal user can Set/DropMaster");
@@ -191,7 +203,7 @@ igt_main
/* Restore the original permissions */
igt_fixture {
- tweak_perm(saved_perm, num, false);
+ tweak_perm(&saved_perm, buf, false);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH i-g-t v2 v2] tests/core_setmaster: simplify device handling
2024-10-04 15:01 [PATCH i-g-t v2 v2] tests/core_setmaster: simplify device handling Bommu Krishnaiah
@ 2024-10-04 15:42 ` Ghimiray, Himal Prasad
0 siblings, 0 replies; 2+ messages in thread
From: Ghimiray, Himal Prasad @ 2024-10-04 15:42 UTC (permalink / raw)
To: Bommu Krishnaiah, igt-dev; +Cc: Emil Velikov, Kamil Konieczny
On 04-10-2024 20:31, Bommu Krishnaiah wrote:
> Refactored test setup to dynamically determine the correct drm card
> for testing. Instead of changing permissions for all cards,
> change only one that will be tested.
>
> Problem Description:
> The test fails after running the `core_hotunplug@hot*` subtest, where
> Card0 is populated as card1, This leads to a failure in the subsequent
> `master-drop-set-user` test.
>
> Command sequence for reproducing the issue:
> - Check available cards: `ls /dev/dri/`
> - Output: by-path card0 renderD128
> - Run the test: `# ./core_hotunplug --r hotrebind-lateclose`
> - Check again: ‘ls /dev/dri/’
> - Output after core_hotunplug : by-path card1 renderD129
> - Run the test: `# ./core_setmaster --r master-drop-set-user`
> - Output: gta@core_setmaster@master-drop-set-user failure
>
> This failures on both XE and i915 for above sequence.
>
> v2: Currented the e-mail address
/s/currented/corrected/
>
> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Kamil Konieczny <kamil.konieczny@intel.com>
> ---
> tests/core_setmaster.c | 82 ++++++++++++++++++++++++------------------
> 1 file changed, 47 insertions(+), 35 deletions(-)
>
> diff --git a/tests/core_setmaster.c b/tests/core_setmaster.c
> index 9c2083f66..fe9993515 100644
> --- a/tests/core_setmaster.c
> +++ b/tests/core_setmaster.c
> @@ -107,40 +107,31 @@ static void check_drop_set(void)
> drm_close_driver(master);
> }
>
> -static unsigned tweak_perm(uint8_t *saved_perm, unsigned max_perm, bool save)
> +static void tweak_perm(uint8_t *saved_perm, char *path, bool save)
> {
> - char path[256];
> struct stat st;
> - unsigned i;
> -
> - for (i = 0; i < max_perm; i++) {
> - snprintf(path, sizeof(path), "/dev/dri/card%u", i);
> -
> - /* Existing userspace assumes there's no gaps, do the same. */
> - if (stat(path, &st) != 0)
> - break;
> -
> - if (save) {
> - /* Save and toggle */
> - saved_perm[i] = st.st_mode & (S_IROTH | S_IWOTH);
> - st.st_mode |= S_IROTH | S_IWOTH;
> - } else {
> - /* Clear and restore */
> - st.st_mode &= ~(S_IROTH | S_IWOTH);
> - st.st_mode |= saved_perm[i];
> - }
> -
> - /* There's only one way for chmod to fail - race vs rmmod.
> - * In that case, do _not_ error/skip, since:
> - * - we need to restore the [correct] permissions
> - * - __drm_open_driver() can open another device, aka the
> - * failure may be irrelevant.
> - */
> - chmod(path, st.st_mode);
> + int ret;
> +
> + if (path[0] == 0)
> + return;
> +
> + ret = stat(path, &st);
> + igt_assert_f(!ret, "stat failed with %d path=%s\n", errno, path);
> +
> + if (save) {
> + /* Save and toggle */
> + *saved_perm = st.st_mode & (S_IROTH | S_IWOTH);
> + st.st_mode |= S_IROTH | S_IWOTH;
> + } else {
> + /* Clear and restore */
> + st.st_mode &= ~(S_IROTH | S_IWOTH);
> + st.st_mode |= *saved_perm;
> }
> - return i;
> -}
>
> + /* There's only one way for chmod to fail - race vs rmmod. */
> + ret = chmod(path, st.st_mode);
> + igt_assert_f(!ret, "chmod failed with %d path=%s\n", errno, path);
> +}
>
> igt_main
> {
> @@ -160,8 +151,8 @@ igt_main
>
>
> igt_subtest_group {
> - uint8_t saved_perm[255];
> - unsigned num;
> + uint8_t saved_perm;
> + char buf[255];
>
> /* Upon dropping root we end up as random user, which
> * a) is not in the video group, and
> @@ -176,8 +167,29 @@ igt_main
> * restored on skip or failure.
> */
> igt_fixture {
> - num = tweak_perm(saved_perm, ARRAY_SIZE(saved_perm),
> - true);
> + char path[255];
> + int len;
> + int fd;
> +
> + memset(buf, 0, sizeof(buf));
> + saved_perm = 0;
> +
> + fd = __drm_open_driver(DRIVER_ANY);
> + igt_assert_fd(fd);
> +
> + snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
> +
> + len = readlink(path, buf, sizeof(buf) - 1);
> + igt_assert_f(len > 0, "readlink failed with %d path=%s\n", errno, path);
> +
> + buf[len] = '\0';
> + if (!(strstr(buf, "/dev/dri/card") == buf ||
> + strstr(buf, "/dev/dri/renderD") == buf))
> + igt_assert_f(0, "Not a card nor render, path=%s\n", buf);
> +
> + igt_assert_eq(__drm_close_driver(fd), 0);
> +
> + tweak_perm(&saved_perm, buf, true);
> }
>
> igt_describe("Ensure first normal user can Set/DropMaster");
> @@ -191,7 +203,7 @@ igt_main
>
> /* Restore the original permissions */
> igt_fixture {
> - tweak_perm(saved_perm, num, false);
> + tweak_perm(&saved_perm, buf, false);
> }
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-10-04 15:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-04 15:01 [PATCH i-g-t v2 v2] tests/core_setmaster: simplify device handling Bommu Krishnaiah
2024-10-04 15:42 ` Ghimiray, Himal Prasad
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox