Re: [PATCH] libtracefs: Destroy synthetic and eprobes before other events

On 16/10/2024 5:35 pm, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <[email protected]>
> 
> Synthetic events can be based on eprobes and other dynamic events. When
> destroying multiple events via tracefs_dynevent_destroy() make sure to
> remove the synthetic events first, followed by eprobes (as they can be on
> other dynamic events as well), then the rest of the dynamic events.
> 
> Signed-off-by: Steven Rostedt (Google) <[email protected]>
> ---
>   src/tracefs-dynevents.c | 42 ++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c
> index 6df7212fb38e..77bdf94fdbfe 100644
> --- a/src/tracefs-dynevents.c
> +++ b/src/tracefs-dynevents.c
> @@ -654,6 +654,22 @@ tracefs_dynevent_get(enum tracefs_dynevent_type type, const char *system,
>   	return devent;
>   }
>   
> +static int destroy_dynevents(struct tracefs_dynevent **all, bool force)
> +{
> +	int ret = 0;
> +	int i;
> +
> +	if (!all)
> +		return 0;
> +
> +	for (i = 0; all[i]; i++) {
> +		if (tracefs_dynevent_destroy(all[i], force))
> +			ret = -1;
> +	}
> +
> +	return ret;
> +}
> +
>   /**
>    * tracefs_dynevent_destroy_all - removes all dynamic events of given types from the system
>    * @types: Dynamic event type, or bitmask of dynamic event types. If 0 is passed, all types
> @@ -671,16 +687,32 @@ int tracefs_dynevent_destroy_all(unsigned int types, bool force)
>   {
>   	struct tracefs_dynevent **all;
>   	int ret = 0;
> -	int i;
>   
> +	/*
> +	 * Destroy synthetic events first, as they may depend on
> +	 * other dynamic events.
> +	 */
> +	if (types & TRACEFS_DYNEVENT_SYNTH) {
> +		all = tracefs_dynevent_get_all(TRACEFS_DYNEVENT_SYNTH, NULL);
> +		ret = destroy_dynevents(all, force);
> +		tracefs_dynevent_list_free(all);
> +		types &= ~TRACEFS_DYNEVENT_SYNTH;
> +	}
> +
> +	/* Eprobes may depend on other events as well */
> +	if (types & TRACEFS_DYNEVENT_EPROBE) {
> +		all = tracefs_dynevent_get_all(TRACEFS_DYNEVENT_EPROBE, NULL);
> +		ret |= destroy_dynevents(all, force);
> +		tracefs_dynevent_list_free(all);
> +		types &= ~TRACEFS_DYNEVENT_EPROBE;
> +	}
> +
> +	/* Destroy the rest */
>   	all = tracefs_dynevent_get_all(types, NULL);
>   	if (!all)
>   		return 0;
>   
> -	for (i = 0; all[i]; i++) {
> -		if (tracefs_dynevent_destroy(all[i], force))
> -			ret = -1;
> -	}
> +	ret |= destroy_dynevents(all, force);
>   
>   	tracefs_dynevent_list_free(all);
>   


I see one more failure in this section of unit tests:

   Test: tracefs_iterate_snapshot_events API ...FAILED
     1. tracefs-utest.c:235  - ret == sizeof(struct test_sample)
     2. tracefs-utest.c:235  - ret == sizeof(struct test_sample)
     3. tracefs-utest.c:235  - ret == sizeof(struct test_sample)

I did run trace-cmd reset and unmounted my tracefs before running the 
unit tests.
Please feel free to ignore if something is weirdly wrong in my setup.

Other than that -kind of existing- failure, the patch looks good to me 
(e.g., trace-cmd unit tests are working fine).