Re: [PATCH i-g-t v5 1/4] runner/resultgen: Added dynamically ignored dmesg messages

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Aug 19, 2024 at 12:32:39PM +0200, Kamil Konieczny wrote:
> Some messages generated by driver are triggered by test itself
> and are not meant to rise error nor warn within runner, yet
> they should be catched in other circumstances so they are not
> suitable to be ignored permanently.
> 
> Instead of hard-coding such situations check dmesg for info from
> a test and add such regex on a fly and then ignore next dmesg
> errors or warns which match it.
> 
> Ignored regex will be removed after end of current subtest, just
> before a new subtest or dynamic subtest starts.
> 
> v2: removed local copy for regex string (Zbigniew), clarify
>  description, free regex before creating new (Kamil)
> v3: print error when compiling regex fails (Zbigniew)
> v4: restore cutting regex string at \n, it was lost in v2
>  (Kamil)
> 
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  runner/output_strings.h |  8 +++++
>  runner/resultgen.c      | 68 ++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 68 insertions(+), 8 deletions(-)
> 
> diff --git a/runner/output_strings.h b/runner/output_strings.h
> index 892895ead..91ef5f2c3 100644
> --- a/runner/output_strings.h
> +++ b/runner/output_strings.h
> @@ -54,6 +54,14 @@ static const char STARTING_SUBTEST_DMESG[] = ": starting subtest ";
>   */
>  static const char STARTING_DYNAMIC_SUBTEST_DMESG[] = ": starting dynamic subtest ";
>  
> +/*
> + * Output in dmesg when a test wants runner to dynamically ignore error or warn.
> + *
> + * Example:
> + * [IGT] add ignored dmesg regex: CRITICAL: Xe has declared device [0-9:.]* as wedged
> + */
> +static const char IGT_ADD_IGNORED_REGEX_DMESG[] = "add ignored dmesg regex: ";
> +
>  /*
>   * Output when a test process is executed.
>   *
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> index 63f5b26d7..87847bf5b 100644
> --- a/runner/resultgen.c
> +++ b/runner/resultgen.c
> @@ -829,23 +829,65 @@ static const char igt_dmesg_whitelist[] =
>  static const char igt_piglit_style_dmesg_blacklist[] =
>  	"(\\[drm:|drm_|intel_|i915_|\\[drm\\])";
>  
> -static bool init_regex_whitelist(struct settings* settings, GRegex **re)
> +static bool init_dmesg_regex(GRegex **re, const char *regex, const char *msg)
>  {
>  	GError *err = NULL;
> -	const char *regex = settings->piglit_style_dmesg ?
> -		igt_piglit_style_dmesg_blacklist :
> -		igt_dmesg_whitelist;
>  
>  	*re = g_regex_new(regex, G_REGEX_OPTIMIZE, 0, &err);
>  	if (err) {
> -		fprintf(stderr, "Cannot compile dmesg regexp\n");
> +		fprintf(stderr, "Cannot compile %s : %s\n",
> +			msg, err->message);
>  		g_error_free(err);
> +
>  		return false;
>  	}
>  
>  	return true;
>  }
>  
> +static bool init_regex_whitelist(struct settings *settings, GRegex **re)
> +{
> +	const char *regex = settings->piglit_style_dmesg ?
> +		igt_piglit_style_dmesg_blacklist :
> +		igt_dmesg_whitelist;
> +	const char *what = settings->piglit_style_dmesg ?
> +		"piglit style dmesg blocklist" :
> +		"igt dmesg whitelist";
> +
> +	return init_dmesg_regex(re, regex, what);
> +}
> +
> +static bool not_ignored(GRegex *re, const char *msg)
> +{
> +	if (!re)
> +		return true;
> +
> +	return !g_regex_match(re, msg, 0, NULL);
> +}
> +
> +static void clean_regex(GRegex **re)
> +{
> +	if (*re)
> +		g_regex_unref(*re);
> +
> +	*re = NULL;
> +}
> +
> +static void add_ignored_regex(GRegex **re, char *src)
> +{
> +	char *s;
> +
> +	s = strchr(src, '\n');
> +	if (s)
> +		*s = 0;
> +
> +	if (*re)
> +		g_regex_unref(*re);
> +
> +	init_dmesg_regex(re, src, "ignore match");
> +	fprintf(stderr, "igt_resultgen: Added ignore regex '%s'\n", src);
> +}
> +
>  static bool parse_dmesg_line(char* line,
>  			     unsigned *flags, unsigned long long *ts_usec,
>  			     char *continuation, char **message)
> @@ -979,6 +1021,7 @@ static bool fill_from_dmesg(int fd,
>  	char dynamic_piglit_name[256];
>  	size_t i;
>  	GRegex *re;
> +	GRegex *re_ignore; /* regex for dynamically ignored dmesg line */
>  
>  	if (!f) {
>  		return false;
> @@ -989,12 +1032,13 @@ static bool fill_from_dmesg(int fd,
>  		return false;
>  	}
>  
> +	re_ignore = NULL;
>  	while (getline(&line, &linelen, f) > 0) {
>  		char *formatted;
>  		unsigned flags;
>  		unsigned long long ts_usec;
>  		char continuation;
> -		char *message, *subtest, *dynamic_subtest;
> +		char *message, *subtest, *dynamic_subtest, *ignore;
>  
>  		if (!parse_dmesg_line(line, &flags, &ts_usec, &continuation, &message))
>  			continue;
> @@ -1024,6 +1068,7 @@ static bool fill_from_dmesg(int fd,
>  			subtest += strlen(STARTING_SUBTEST_DMESG);
>  			generate_piglit_name(binary, subtest, piglit_name, sizeof(piglit_name));
>  			current_test = get_or_create_json_object(tests, piglit_name);
> +			clean_regex(&re_ignore);
>  		}
>  
>  		if (current_test != NULL &&
> @@ -1041,18 +1086,24 @@ static bool fill_from_dmesg(int fd,
>  			dynamic_subtest += strlen(STARTING_DYNAMIC_SUBTEST_DMESG);
>  			generate_piglit_name_for_dynamic(piglit_name, dynamic_subtest, dynamic_piglit_name, sizeof(dynamic_piglit_name));
>  			current_dynamic_test = get_or_create_json_object(tests, dynamic_piglit_name);
> +			clean_regex(&re_ignore);
>  		}
>  
> +		if ((ignore = strstr(message, IGT_ADD_IGNORED_REGEX_DMESG)) != NULL)
> +			add_ignored_regex(&re_ignore, ignore + strlen(IGT_ADD_IGNORED_REGEX_DMESG));
> +
>  		if (settings->piglit_style_dmesg) {
>  			if ((flags & 0x07) <= settings->dmesg_warn_level && continuation != 'c' &&
> -			    g_regex_match(re, message, 0, NULL)) {
> +			    g_regex_match(re, message, 0, NULL) &&
> +			    not_ignored(re_ignore, message)) {
>  				append_line(&warnings, &warningslen, formatted);
>  				if (current_test != NULL)
>  					append_line(&dynamic_warnings, &dynamic_warnings_len, formatted);
>  			}
>  		} else {
>  			if ((flags & 0x07) <= settings->dmesg_warn_level && continuation != 'c' &&
> -			    !g_regex_match(re, message, 0, NULL)) {
> +			    !g_regex_match(re, message, 0, NULL) &&
> +			    not_ignored(re_ignore, message)) {
>  				append_line(&warnings, &warningslen, formatted);
>  				if (current_test != NULL)
>  					append_line(&dynamic_warnings, &dynamic_warnings_len, formatted);
> @@ -1099,6 +1150,7 @@ static bool fill_from_dmesg(int fd,
>  	free(dynamic_dmesg);
>  	free(warnings);
>  	free(dynamic_warnings);
> +	clean_regex(&re_ignore);
>  	g_regex_unref(re);
>  	fclose(f);
>  	return true;
> -- 
> 2.43.0
>