[PATCH v2] tests/core_setmaster: Handle the test even if cards are non-continuous

[Kamil Konieczny]

Hi Bommu,
On 2024-09-10 at 11:07:47 +0530, Bommu Krishnaiah wrote:
> Although most of user space assumes drm cards to be populated continuously,
> this assumption may not be always true. After hot unpluging and repluging
> of card, the card may be non-continuous. To address such scenarios where
> the cards can be non-continuous Modify the tweak_perm function to loop all
> possible card numbers(card0-card255) and break after first card found.
> 
> Problem Description:
> The test fails after running the `core_hotunplug at 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 at master-drop-set-user failure
> 
> This failures on both XE and i915 for above sequence.
> 
> v2: Break the loop after first card found and check return value of chmod.
> 
> Signed-off-by: Bommu Krishnaiah krishnaiah.bommu at intel.com
> Cc: Emil Velikov emil.l.velikov at gmail.com
> Cc: Himal Prasad Ghimiray himal.prasad.ghimiray at intel.com
> Cc: Kamil Konieczny kamil.konieczny at linux.intel.com
> ---
>  tests/core_setmaster.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/core_setmaster.c b/tests/core_setmaster.c
> index 9c2083f66..bfcfb5d25 100644
> --- a/tests/core_setmaster.c
> +++ b/tests/core_setmaster.c
> @@ -107,18 +107,20 @@ static void check_drop_set(void)
>  	drm_close_driver(master);
>  }
>  
> -static unsigned tweak_perm(uint8_t *saved_perm, unsigned max_perm, bool save)
> +static int tweak_perm(uint8_t *saved_perm, unsigned max_perm, bool save)
>  {
>  	char path[256];
>  	struct stat st;
> -	unsigned i;
> +	int i;
>  
>  	for (i = 0; i < max_perm; i++) {
> +		int ret = 0;
> +
>  		snprintf(path, sizeof(path), "/dev/dri/card%u", i);
>  
>  		/* Existing userspace assumes there's no gaps, do the same. */

Please update comment or refactor whole test:

use pair of __drm_open_driver/__drm_close_driver,

imho something like:
in first fixup open fd and get its path, this will allow to
eliminate loop,
then after succesfully changing permissions set also at_exit() proc
for restoring priviliges if test fails.

if changing permissions fails just fail with igt_assert()

Regards,
Kamil

>  		if (stat(path, &st) != 0)
> -			break;
> +			continue;
>  
>  		if (save) {
>  			/* Save and toggle */
> @@ -136,7 +138,12 @@ static unsigned tweak_perm(uint8_t *saved_perm, unsigned max_perm, bool save)
>  		 * - __drm_open_driver() can open another device, aka the
>  		 * failure may be irrelevant.
>  		 */
> -		chmod(path, st.st_mode);
> +		ret = chmod(path, st.st_mode);
> +		if (ret)
> +			return ret;
> +
> +		if (save)
> +			return i;
>  	}
>  	return i;
>  }
> @@ -161,7 +168,7 @@ igt_main
>  
>  	igt_subtest_group {
>  		uint8_t saved_perm[255];
> -		unsigned num;
> +		int num;
>  
>  		/* Upon dropping root we end up as random user, which
>  		 * a) is not in the video group, and
> -- 
> 2.25.1
>