Re: [PATCH v4 i-g-t 2/3] lib/igt_kmod: Add IGT_HOOK_POST_KMOD_UNBIND

[Peter Senna Tschudin <peter.senna@linux.intel.com>]

On 7/23/2025 7:01 PM, Kamil Konieczny wrote:
> Hi Peter,
> On 2025-07-23 at 12:44:05 +0200, Peter Senna Tschudin wrote:
>> Adds an igt_hook after the unbind operation of the kmod intended as
>> a strategice entry point for kmemleak scans.
>>
>> Can be used with --hook  'post-kmod-unbind:...'
>>
>> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>> Signed-off-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
>> ---
>> v4:
>>  - unchanged from v3
> 
> You got r-b from Gustavo so imho you could add his r-b here.
> Do I miss something? Or do you plan on merging this with last patch?

I forgot to add it to v4, but the r-b is still valid as nothing changed.
So here it is from:
https://patchwork.freedesktop.org/patch/664129/?series=151642&rev=1

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

> 
> Regards,
> Kamil
> 
>>
>> v3:
>>   - Renamed the hook to IGT_HOOK_POST_KMOD_UNBIND as the unbind can also happen
>>    for snd modules
>>  - Changed the logic for the IGT_HOOK_KMOD_UNBIND_MODULE_NAME env var so that
>>    it is only populated when the correct hook is used.
>>
>> v2:
>>  - Added code ducumentation for new hook
>>  - Added env var IGT_HOOK_KMOD_UNBIND_MODULE_NAME and extended help text
>>
>>  lib/igt_hook.c | 11 +++++++++++
>>  lib/igt_hook.h |  3 +++
>>  lib/igt_kmod.c |  8 ++++++++
>>  3 files changed, 22 insertions(+)
>>
>> diff --git a/lib/igt_hook.c b/lib/igt_hook.c
>> index 8932d118d..f86ed56f7 100644
>> --- a/lib/igt_hook.c
>> +++ b/lib/igt_hook.c
>> @@ -74,6 +74,8 @@ static const char *igt_hook_evt_type_to_name(enum igt_hook_evt_type evt_type)
>>  		return "post-subtest";
>>  	case IGT_HOOK_POST_TEST:
>>  		return "post-test";
>> +	case IGT_HOOK_POST_KMOD_UNBIND:
>> +		return "post-kmod-unbind";
>>  	case IGT_HOOK_NUM_EVENTS:
>>  		break;
>>  	/* No "default:" case, to force a warning from -Wswitch in case we miss
>> @@ -335,6 +337,8 @@ static void igt_hook_update_env_vars(struct igt_hook *igt_hook, struct igt_hook_
>>  	setenv("IGT_HOOK_TEST", igt_hook->test_name, 1);
>>  	setenv("IGT_HOOK_SUBTEST", igt_hook->subtest_name, 1);
>>  	setenv("IGT_HOOK_DYN_SUBTEST", igt_hook->dyn_subtest_name, 1);
>> +	setenv("IGT_HOOK_KMOD_UNBIND_MODULE_NAME",
>> +	       evt->evt_type == IGT_HOOK_POST_KMOD_UNBIND ? evt->target_name : "", 1);
>>  	setenv("IGT_HOOK_RESULT", evt->result ?: "", 1);
>>  }
>>  
>> @@ -470,6 +474,9 @@ below:\n\
>>  		case IGT_HOOK_POST_TEST:
>>  			desc = "Occurs after a test case has finished.";
>>  			break;
>> +		case IGT_HOOK_POST_KMOD_UNBIND:
>> +			desc = "Occurs after the kernel module is unbound from the device.";
>> +			break;
>>  		default:
>>  			desc = "MISSING DESCRIPTION";
>>  		}
>> @@ -504,6 +511,10 @@ available to the command:\n\
>>    values are: SUCCESS, SKIP or FAIL. This is only applicable on \"post-*\"\n\
>>    events and will be the empty string for other types of events.\n\
>>  \n\
>> +  IGT_HOOK_KMOD_UNBIND_MODULE_NAME\n\
>> +  Name of the kernel module that was unbound from the device. This is only\n\
>> +  applicable on the `IGT_HOOK_POST_KMOD_UNBIND` event and will be the empty\n\
>> +  string for other types of events.\n\
>>  \n\
>>  Note that %s can be passed multiple times. Each descriptor is evaluated in turn\n\
>>  when matching events and running hook commands.\n\
>> diff --git a/lib/igt_hook.h b/lib/igt_hook.h
>> index e9f97b79b..11d839dfd 100644
>> --- a/lib/igt_hook.h
>> +++ b/lib/igt_hook.h
>> @@ -26,6 +26,8 @@ struct igt_hook;
>>   * @IGT_HOOK_POST_SUBTEST: Occurs after the execution of a subtest..
>>   * @IGT_HOOK_POST_TEST: Occurs after a test case (executable) is finished with
>>   * the test code.
>> + * @IGT_HOOK_POST_KMOD_UNBIND: Occurs after the kernel module is unbound
>> + * from the device.
>>   * @IGT_HOOK_NUM_EVENTS: This is not really an event and represents the number
>>   * of possible events tracked by igt_hook.
>>   *
>> @@ -39,6 +41,7 @@ enum igt_hook_evt_type {
>>  	IGT_HOOK_POST_DYN_SUBTEST,
>>  	IGT_HOOK_POST_SUBTEST,
>>  	IGT_HOOK_POST_TEST,
>> +	IGT_HOOK_POST_KMOD_UNBIND,
>>  	IGT_HOOK_NUM_EVENTS /* This must always be the last one. */
>>  };
>>  
>> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
>> index 6d7f5705c..a10626eed 100644
>> --- a/lib/igt_kmod.c
>> +++ b/lib/igt_kmod.c
>> @@ -41,6 +41,7 @@
>>  #include "igt_aux.h"
>>  #include "igt_core.h"
>>  #include "igt_debugfs.h"
>> +#include "igt_hook.h"
>>  #include "igt_kmod.h"
>>  #include "igt_ktap.h"
>>  #include "igt_sysfs.h"
>> @@ -604,6 +605,7 @@ int __igt_intel_driver_unload(char **who, const char *driver)
>>   */
>>  int igt_kmod_unbind(const char *mod_name, const char *pci_device)
>>  {
>> +	struct igt_hook *igt_hook = NULL;
>>  	char path[PATH_MAX];
>>  	struct dirent *de;
>>  	int dirlen;
>> @@ -634,6 +636,12 @@ int igt_kmod_unbind(const char *mod_name, const char *pci_device)
>>  
>>  	closedir(dir);
>>  
>> +	igt_hook = igt_core_get_igt_hook();
>> +	igt_hook_event_notify(igt_hook, &(struct igt_hook_evt){
>> +		.evt_type = IGT_HOOK_POST_KMOD_UNBIND,
>> +		.target_name = mod_name,
>> +	});
>> +
>>  	return 0;
>>  }
>>  
>> -- 
>> 2.43.0
>>