[PATCH i-g-t] tests/kms_cursor_legacy: Reduce debug loglevel dynamically

[Wheeler, Daniel]

[Public]

Hey just checking  in to see if there has been any movement on this one. I'm still having to revert it from my local IGT repo or my runs take three times as long to complete/logs end up 10,000 times larger than they should be.

Thank you,

Dan Wheeler
Sr. Technologist  |  AMD
SW Display
------------------------------------------------------------------------------------------------------------------
1 Commerce Valley Dr E, Thornhill, ON L3T 7X6
Facebook |  Twitter |  amd.com


-----Original Message-----
From: Jani Nikula <jani.nikula at intel.com>
Sent: Monday, February 10, 2025 10:58 AM
To: Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Pranay Samala <pranay.samala at intel.com>; igt-dev at lists.freedesktop.org
Cc: karthik.b.s at intel.com; kunal1.joshi at intel.com; sameer.lattannavar at intel.com; Wheeler, Daniel <Daniel.Wheeler at amd.com>; Bhanuprakash Modem <bhanuprakash.modem at intel.com>; Kamil Konieczny <kamil.konieczny at linux.intel.com>; Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>; Ashutosh Dixit <ashutosh.dixit at intel.com>
Subject: Re: [PATCH i-g-t] tests/kms_cursor_legacy: Reduce debug loglevel dynamically

On Mon, 10 Feb 2025, Leo Li <sunpeng.li at amd.com> wrote:
> Hi Pranay,
>
> This change can also increase log verbosity; we observe this with amdgpu.
> Details inline.
>
> On 2025-01-28 00:28, Pranay Samala wrote:
>> This test is getting killed due to exceeding disk usage limit on CI
>> environment because its logging to many logs.
>>
>> So dynamically reducing the debug log level to 10.
>>
>> Signed-off-by: Pranay Samala <pranay.samala at intel.com>
>> ---
>>   tests/kms_cursor_legacy.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
>> index 1f082df2d..533972cd4 100644
>> --- a/tests/kms_cursor_legacy.c
>> +++ b/tests/kms_cursor_legacy.c
>> @@ -40,6 +40,7 @@
>>   #include "igt_psr.h"
>>   #include "igt_rand.h"
>>   #include "igt_stats.h"
>> +#include "igt_sysfs.h"
>>
>>   /**
>>    * SUBTEST: %s-%s
>> @@ -1840,6 +1841,7 @@ igt_main
>>      };
>>
>>      igt_fixture {
>> +            int dir, current_log_level;
>>              display.drm_fd = drm_open_driver_master(DRIVER_ANY);
>>              kmstest_set_vt_graphics_mode();
>>
>> @@ -1850,6 +1852,15 @@ igt_main
>>               * fetch is enabled, so switching PSR1 for this whole test.
>>               */
>>              intel_psr2_restore = i915_psr2_sel_fetch_to_psr1(display.drm_fd,
>> NULL);
>> +
>> +            dir = igt_sysfs_drm_module_params_open();
>> +            if (dir >= 0) {
>> +                    current_log_level = igt_drm_debug_level_get(dir);
>> +                    close(dir);
>> +
>> +                    if (current_log_level > 10)
>> +                            igt_drm_debug_level_update(10);
>
> Isn't the DRM debug mask a bitmask?
> https://elixir.bootlin.com/linux/v6.13.1/source/include/drm/drm_print.
> h#L97
>
> So setting 10 isn't guaranteed to reduce verbosity, depending on how
> the DRM driver uses the various drm_* debug prints.

Ugh. Yes.

> What is guaranteed to reduce verbosity is to unset unwanted bits.
> Maybe that is the better approach?
>
> For example, instead of
> `if (current_log_level > 10)`,
>
> Do
>
> `igt_drm_debug_level_update(current_log_level & !(DRM_UT_CORE | ...);`

Ditto for commit a2ab0ec12ef4 ("tests/kms_atomic_transition: Reducing debug loglevel dynamically"). It got to v6 and nobody noticed?

Ditto for commit 56b91193b825 ("lib/igt_sysfs: Implement dynamic adjustment of debug log level") adding the whole thing. It should not be referred to as "level" because it has nothing to do with "level". It also makes no sense to print it out in decimal; hex makes more sense.


BR,
Jani.


>
> Thanks,
> Leo
>
>> +            }
>>      }
>>
>>      igt_describe("Test checks how many cursor updates we can fit between vblanks "
>

--
Jani Nikula, Intel