unikraft / lib-musl

musl: A C standard library
Other
8 stars 27 forks source link

Initialize main thread pthread structure #59

Closed alexhoppus closed 1 year ago

alexhoppus commented 1 year ago

It seems some steps were missing in pthread structure initialization. This patch does the following:

alexhoppus commented 1 year ago

previously discussed here https://github.com/unikraft/unikraft/pull/961#pullrequestreview-1498831909

regarding joinable for main thread please grep here https://pubs.opengroup.org/onlinepubs/009695399/functions/exec.html

razvand commented 1 year ago

@alexhoppus , @kubanrob , could you please ensure this PR is updated in the next days to have it part of the 0.14 release?

kubanrob commented 1 year ago

@razvand To be honest I am not sure what exactly needs to be changed given @alexhoppus and @andreittr answers.

@skuenzer Please review again given the answers. To me it seems like the requested three changes are not really needed / desired.

macro

@skuenzer

  • Is sizeof(void *) big enough to hold a TSD entry?
  • Isn't somewhere else a size definition that we should use instead (to avoid code-out-of-sync type of errors in the future)?

@alexhoppus

Actually, I have not introduced this, that was before me, I just moved this line from below. That is how TSD was created for other threads.

@andreittr

macro vs static const: I think it hardly matters; in general I'd go for the option of least change.

TSD

@skuenzer

Do we need to allocate TSD here already? The dangerous part is, because __uk_copy_tls() is called on every TLS memory allocation that we crash at any TLS creation

@alexhoppus

uk_copy_tls() relates to this. It is in uk_init_tp()

@andreittr

uk_copy_tls indeed it's not relevant; all the changed code is in uk_init_tp which gets called only for the main thread.

andreittr commented 1 year ago

@razvand To be honest I am not sure what exactly needs to be changed given @alexhoppus and @andreittr answers.

No changes per se, but there are some pending questions in the latest comment from @skuenzer and in my review above. @alexhoppus could you please have a look?

alexhoppus commented 1 year ago

but there are some pending questions in the latest comment from @skuenzer and in my review above.

@andreittr I answered about error propagation and macro vs static const questions. I don't see any other questions. Please let me know if I missed something. Thank you.

andreittr commented 1 year ago

but there are some pending questions in the latest comment from @skuenzer and in my review above.

@andreittr I answered about error propagation and macro vs static const questions. I don't see any other questions. Please let me know if I missed something. Thank you.

Oops, my bad, I had left my review in draft form and it wouldn't show to others; fixed now, my 2 questions should now be visible.