Re: [Minios-devel] [UNIKRAFT/PTHREAD-EMBEDDED PATCH 1/1] Set highest priority to pthread constructor

[Vlad-Andrei BĂDOIU (78692) <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>]

Hey Simon,

I had a discussion with Costin on this topic before reviewing the patch 
and commenting on it being a temporal fix. Using another class of 
constructors for pthread would not solve the underlying problem. In this 
case, we presume that there exists at least one other library that needs 
to be initiated before pthreads; We end up with the same issue. In my 
opinion, a way to solve this would be to add an order operator for 
constructors. Let me know what you think.

Thanks,

Vlad

On 27.11.2019 10:45, Simon Kuenzer wrote:
> Any comments?
>
> Thanks,
>
> Simon
>
> On 11.11.19 15:19, Simon Kuenzer wrote:
>> Oh sorry, these questions are actually going to the related patch of 
>> to Unikraft's main repository that introduces /include/uk/ctors_prio.h:
>>   GIT d56a47c2d
>>   Patchwork https://patchwork.unikraft.org/patch/736664/
>>
>> I understand that this is supposed to be a temporary fix. I just 
>> wonder what was missing on the existing code base to make the 
>> temporary solution work. I see pthread_embedded as a basic system 
>> lib, so it would be fine to use the system constructors or the 
>> inittable depending when the functionality has to be initialized. 
>> Both of them are called before the application constructors are called.
>>
>> Thanks,
>>
>> Simon
>>
>> On 11.11.19 14:53, Simon Kuenzer wrote:
>>> Hey guys,
>>>
>>> I noticed that this patch went in already, however, I still have a 
>>> few questions to this:
>>>
>>> * Why isn't the content of uk/ctors_prio.h defined in ctors.h? We 
>>> also have already a constructor attribute helper with priority level 
>>> in essentials.h...
>>> * Why is needed to define a priority level of an external library in 
>>> the Unikraft repo?
>>> * And isn't 101, too late? Shouldn't the call go into the uk_ctortab 
>>> (before sched and allocator) or uk_inittab (after sched and 
>>> allocator but before application constructors -> e.g., 
>>> uk_lib_initcall)?
>>>
>>> Thanks,
>>>
>>> Simon
>>>
>>> On 01.11.19 07:55, Costin Lupu wrote:
>>>> Hi Vlad,
>>>>
>>>> I will replace 101 with UK_CTOR_PRIO_PTHREAD_EMBEDDED on upstreaming.
>>>>
>>>> Thanks,
>>>> Costin
>>>>
>>>> Reviewed-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>>>>
>>>> On 10/31/19 12:37 AM, Vlad-Andrei BĂDOIU (78692) wrote:
>>>>> Other libraries might use the default constructor attribute and 
>>>>> expect pthreads
>>>>> to be initialised. We set the highest priority(101) to the pthread 
>>>>> constructor.
>>>>>
>>>>> Signed-off-by: Vlad-Andrei Badoiu 
>>>>> <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
>>>>> ---
>>>>>   pte_osal.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/pte_osal.c b/pte_osal.c
>>>>> index 23dcd01..df24bed 100644
>>>>> --- a/pte_osal.c
>>>>> +++ b/pte_osal.c
>>>>> @@ -58,7 +58,7 @@ typedef struct {
>>>>>    *
>>>>> ***************************************************************************/
>>>>>  
>>>>>
>>>>> -int __constructor pthread_constructor(void)
>>>>> +int __attribute__((constructor(101))) pthread_constructor(void)
>>>>>   {
>>>>>       uk_pr_debug("pthread-embedded constructor\n");
>>>>>       return pthread_init();
>>>>>
>>>>
>>>> _______________________________________________
>>>> Minios-devel mailing list
>>>> Minios-devel@xxxxxxxxxxxxxxxxxxxx
>>>> https://lists.xenproject.org/mailman/listinfo/minios-devel
>>>>
>>>
>>> _______________________________________________
>>> Minios-devel mailing list
>>> Minios-devel@xxxxxxxxxxxxxxxxxxxx
>>> https://lists.xenproject.org/mailman/listinfo/minios-devel
>>
>> _______________________________________________
>> Minios-devel mailing list
>> Minios-devel@xxxxxxxxxxxxxxxxxxxx
>> https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel