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

[Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>]

Hi Costin,

On Mon, 2020-03-02 at 12:31 +0200, Costin Lupu wrote:
> 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.

I have ported Mimalloc, a new dynamic memory allocator[0], whose
initialization requires a functioning pthread. This makes Mimalloc's
port a little bit trickier, because the allocator is typically
initialized before pthread. Besides, pthread's initialization itself
requires a functioning allocator.

I have addressed this issue by providing an efficient, yet primitive
allocator to satisfy allocations before pthread's intialization. I will
call this the "early boot time". The transition to Mimalloc is
triggered as soon as pthread has been initialized.

In order to do this, I need to know when pthread-embedded has been
initialized, and most importantly when the TLS is initialized and
usable. This is what this patch initially intended to do. This check is
be done in the early boot time malloc() implementation and should be as
simple as possible to avoid additional overhead.

Obviously this patch is not a good solution, at the very least because
this function does not guarantee that pte_osInit returned, and for all
the reasons you mentioned in your message.

Checking for (current_thread)->prv != NULL now looks like a better
option to me. This will become true at the end of pte_osInit,
indicating that the TLS has been successfully initialized.

thanks for your comments!

regards,
Hugo

[0] https://github.com/microsoft/mimalloc

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