[PATCH] btf: ensure BTF-to-CTF conversion works for modules

[PATCH] btf: ensure BTF-to-CTF conversion works for modules

[Kris Van Hees]

The recent introduction of code to always load BTF data for modules
broke the BTF-to-CTF conversion for modules.  This patch ensures that
the BTF-to-CTF convertor code is called from any module that does not
contain CTF data.

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
 libdtrace/dt_module.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libdtrace/dt_module.c b/libdtrace/dt_module.c
index 5e608446..cbbb1d3c 100644
--- a/libdtrace/dt_module.c
+++ b/libdtrace/dt_module.c
@@ -962,8 +962,8 @@ dt_kern_module_find_ctf(dtrace_hdl_t *dtp, dt_module_t *dmp)
 			dt_dprintf("Cannot open CTF archive %s: %s; "
 				   "looking for in-module CTF instead.\n",
 				   ctfa_name, ctf_errmsg(dtp->dt_ctferr));
-#endif
 			return;
+#endif
 		}
 
 		if (dtp->dt_ctfa_path == NULL)
@@ -1018,6 +1018,11 @@ dt_kern_module_find_ctf(dtrace_hdl_t *dtp, dt_module_t *dmp)
 
 		dmp->dm_flags |= DT_DM_CTF_ARCHIVED;
 		ctf_setspecific(dmp->dm_ctfp, dmp);
+#ifdef HAVE_LIBCTF
+	} else {
+		/* Generate CTF from BTF for the module. */
+		dt_kern_module_ctf_from_btf(dtp, dmp);
+#endif
 	}
 
 	/*
-- 
2.42.0

Re: [DTrace-devel] [PATCH] btf: ensure BTF-to-CTF conversion works for modules

[Alan Maguire]

On 25/07/2024 05:56, Kris Van Hees via DTrace-devel wrote:
> The recent introduction of code to always load BTF data for modules
> broke the BTF-to-CTF conversion for modules.  This patch ensures that
> the BTF-to-CTF convertor code is called from any module that does not
> contain CTF data.
> 
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>

Tested-by: Alan Maguire <alan.maguire@oracle.com>

Tested with patches 1 and 3 from [1] applied.

[1]
https://lore.kernel.org/dtrace/20240708145404.1673045-1-alan.maguire@oracle.com/


> ---
>  libdtrace/dt_module.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libdtrace/dt_module.c b/libdtrace/dt_module.c
> index 5e608446..cbbb1d3c 100644
> --- a/libdtrace/dt_module.c
> +++ b/libdtrace/dt_module.c
> @@ -962,8 +962,8 @@ dt_kern_module_find_ctf(dtrace_hdl_t *dtp, dt_module_t *dmp)
>  			dt_dprintf("Cannot open CTF archive %s: %s; "
>  				   "looking for in-module CTF instead.\n",
>  				   ctfa_name, ctf_errmsg(dtp->dt_ctferr));
> -#endif
>  			return;
> +#endif
>  		}
>  
>  		if (dtp->dt_ctfa_path == NULL)
> @@ -1018,6 +1018,11 @@ dt_kern_module_find_ctf(dtrace_hdl_t *dtp, dt_module_t *dmp)
>  
>  		dmp->dm_flags |= DT_DM_CTF_ARCHIVED;
>  		ctf_setspecific(dmp->dm_ctfp, dmp);
> +#ifdef HAVE_LIBCTF
> +	} else {
> +		/* Generate CTF from BTF for the module. */
> +		dt_kern_module_ctf_from_btf(dtp, dmp);
> +#endif
>  	}
>  
>  	/*

Re: [DTrace-devel] [PATCH] btf: ensure BTF-to-CTF conversion works for modules

[Nick Alcock]

On 25 Jul 2024, Kris Van Hees via DTrace-devel verbalised:

> The recent introduction of code to always load BTF data for modules
> broke the BTF-to-CTF conversion for modules.  This patch ensures that
> the BTF-to-CTF convertor code is called from any module that does not
> contain CTF data.
>
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>  libdtrace/dt_module.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libdtrace/dt_module.c b/libdtrace/dt_module.c
> index 5e608446..cbbb1d3c 100644
> --- a/libdtrace/dt_module.c
> +++ b/libdtrace/dt_module.c
> @@ -962,8 +962,8 @@ dt_kern_module_find_ctf(dtrace_hdl_t *dtp, dt_module_t *dmp)
>  			dt_dprintf("Cannot open CTF archive %s: %s; "
>  				   "looking for in-module CTF instead.\n",
>  				   ctfa_name, ctf_errmsg(dtp->dt_ctferr));
> -#endif
>  			return;
> +#endif
>  		}

I'm having to refresh old memories here, so this could be wrong, but...

... this hunk seems wrong to me. Why is this return here at all? The
original implementation (before the converter) tries to load the shared
CTF from a variety of places (of which this is just the first) then goes
on to try to load standalone module CTF if the CTF for this module isn't
present in it. With that return in there, if the CTFA isn't found we
fail to look for CTF at all on the libdtrace-ctf (!HAVE_LIBCTF) pathway,
which is precisely the time in which some of those fallbacks (.ko.ctf
etc) may be needed.

Why not just drop the return?

>  		if (dtp->dt_ctfa_path == NULL)
> @@ -1018,6 +1018,11 @@ dt_kern_module_find_ctf(dtrace_hdl_t *dtp, dt_module_t *dmp)
>  
>  		dmp->dm_flags |= DT_DM_CTF_ARCHIVED;
>  		ctf_setspecific(dmp->dm_ctfp, dmp);
> +#ifdef HAVE_LIBCTF
> +	} else {
> +		/* Generate CTF from BTF for the module. */
> +		dt_kern_module_ctf_from_btf(dtp, dmp);
> +#endif
>  	}

This should probably be one conditional later, so that we only build
from BTF if there is no out-of-tree module with CTF in it. (i.e. at the
very end.)

-- 
NULL && (void)