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

[Kamil Konieczny]

Hi Bommu,
On 2024-08-12 at 10:54:43 +0530, Bommu Krishnaiah wrote:
> Although most of userspace assumes drm cards to be populated continously,
> 

Remove blank line.

> this assumption may not be always true. After hot unpluging and
> repluging of card
> 

Remove blank.

> the card may be non-continous.  To address such scenarios where the
> cards can be non-continous
> 

Remove blank.

> modify the tweak_perm function to loop all possible
> cardnumbers(card0-card255) to confirm are they populated or not.
-----^
s/cardnumbers/card numbers/

imho test is checking only first card so looping all is not optimal.

> 
> Problem Description:
> The test fails after running the `core_hotunplug at hot*` subtest, where
> card0 is missing.

imho this could be discussed if that is ok or driver error,
imho valid case would be to have multi-card and unpluging first.

> 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 failure: by-path  card1  renderD129

s/failure/core_hotunplug failure/

> - Run the test: `# ./core_setmaster --r master-drop-set-user`
> 
> This failures on both XE and i915 for above sequence.
> 
> 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 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/core_setmaster.c b/tests/core_setmaster.c
> index 9c2083f66..a2758e2f2 100644
> --- a/tests/core_setmaster.c
> +++ b/tests/core_setmaster.c
> @@ -116,9 +116,10 @@ static unsigned tweak_perm(uint8_t *saved_perm, unsigned max_perm, bool save)
>  	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. */
> +		/* Userspace cannot always have continuous card0...N due to
> +		 * dynamic unloading or removal of cards */
>  		if (stat(path, &st) != 0)
> -			break;
> +			continue;

imho this is ok but you should also break after first save/restore
just before end of for loop, so no need for setting all perms when
you will test only first one.

Also imho you should fill info in saved_perm about which card was
found and use it instead of calling __drm_open_driver().

Regards,
Kamil

>  
>  		if (save) {
>  			/* Save and toggle */
> -- 
> 2.25.1
>