Re: [Minios-devel] [UNIKRAFT/LIBPTHREAD-EMBEDDED PATCH] tls-helper: add pteTlsGlobalInitialized

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Hugo,

I have some comments about your changes.

1. What are you trying to achieve here?
a. You want to know whether there is TLS support? If so, this should be
a feature provided by the kernel.
b. You want to know whether the TLS structures in pthread-embedded were
initialized? If so, why?

2. pte* functions are internal to pthread-embedded and they *must* not
be used outside of it.

3. If you only introduce a new function, putting it in a patch is not
the best way to do it. You can simply add it to the pthread-embedded
glue code. As a rule of thumb, you should avoid adding patches as much
as you can. Patches to original code are always the last resort.

4. In general, the commit messages should be as clear as possible about
the reasons behind the changes being introduced.

Cheers,
Costin

On 3/2/20 11:41 AM, Hugo Lefeuvre wrote:
> Used in Mimalloc's port to detect availability of TLS.
> 
> Signed-off-by: Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>
> ---
>  patches/0011-add-pteTlsGlobalInitialized.patch | 26 
> ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 patches/0011-add-pteTlsGlobalInitialized.patch
> 
> diff --git a/patches/0011-add-pteTlsGlobalInitialized.patch 
> b/patches/0011-add-pteTlsGlobalInitialized.patch
> new file mode 100644
> index 0000000..bb967c5
> --- /dev/null
> +++ b/patches/0011-add-pteTlsGlobalInitialized.patch
> @@ -0,0 +1,26 @@
> +diff -urNp pthread-patched/platform/helper/tls-helper.c 
> pthread-dev/platform/helper/tls-helper.c
> +--- pthread-patched/platform/helper/tls-helper.c     2020-02-28 
> 09:25:46.922415709 +0100
> ++++ pthread-dev/platform/helper/tls-helper.c 2020-03-01 14:42:41.856370103 
> +0100
> +@@ -38,6 +38,11 @@ static int maxTlsValues;
> + 
> + pte_osMutexHandle globalTlsLock;
> + 
> ++int pteTlsGlobalInitialized()
> ++{
> ++  return keysUsed != NULL;
> ++}
> ++
> + pte_osResult pteTlsGlobalInit(int maxEntries)
> + {
> +   int i;
> +diff -urNp pthread-patched/platform/helper/tls-helper.h 
> pthread-dev/platform/helper/tls-helper.h
> +--- pthread-patched/platform/helper/tls-helper.h     2020-02-28 
> 09:25:46.926416159 +0100
> ++++ pthread-dev/platform/helper/tls-helper.h 2020-03-01 14:43:05.989262693 
> +0100
> +@@ -34,6 +34,7 @@
> + /// @todo document..
> + 
> + pte_osResult pteTlsGlobalInit(int maxEntries);
> ++int pteTlsGlobalInitialized();
> + void * pteTlsThreadInit(void);
> + 
> + pte_osResult pteTlsAlloc(unsigned int *pKey);
> 

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel