[PATCH v2] c-user: Document new clock manager directives

[Sebastian Huber]

On 17/11/2021 19:32, Sebastian Huber wrote:
> On 16/11/2021 22:47, Chris Johns wrote:
>>
>> On 16/11/21 4:27 am, Sebastian Huber wrote:
>>> On 11/11/2021 08:02, Sebastian Huber wrote:> On 09/11/2021 13:06, 
>>> Sebastian
>>> Huber wrote:
>>>>> On 09/11/2021 08:50, Sebastian Huber wrote:
>>>>>> On 09/11/2021 08:41, Chris Johns wrote:
>>>>>>>> We could also use something like this:
>>>>>>>>
>>>>>>>> static inline struct timespec rtems_clock_get_realtime(void)
>>>>>>>> {
>>>>>>>>     struct timespec time_snapshot;
>>>>>>>>
>>>>>>>>     _Timecounter_Nanotime( &time_snapshot );
>>>>>>>>
>>>>>>>>     return time_snapshot;
>>>>>>>> }
>>>>>>>>
>>>>>>>> Unfortunately GCC is not able to optimize this.
>>>>>>>>
>>>>>>> Ah OK. This can be fixed and the performance improved but once 
>>>>>>> the API is
>>>>>>> set it
>>>>>>> cannot change or do you think we can add a check later and not 
>>>>>>> break the API?
>>>>>> I filed a GCC bug for this:
>>>>>>
>>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103150
>>>>>>
>>>>>> It seems I was not the only one noticing issues related to 
>>>>>> structure returns:
>>>>>>
>>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101926
>>>>>>
>>>>>> However, if we want a foolproof API, then I would prefer the 
>>>>>> structure
>>>>>> return over the return status and pointer argument. Compilers may 
>>>>>> get better
>>>>>> in the future. clang has similar issues, so this is not only a GCC 
>>>>>> problem.
>>>>> We have at least three options for the API:
>>>>>
>>>>> 1. Use the existing FreeBSD implementation as is:
>>>>>
>>>>> void rtems_clock_get_realtime(struct timespec *);
>>>>>
>>>>> This is the easiest and most efficient approach.
>>>>>
>>>>> 2. Check for NULL and return a status:
>>>>>
>>>>> rtems_status_code rtems_clock_get_realtime(struct timespec *);
>>>>>
>>>>> This requires a wrapper function which is a bit less efficient and 
>>>>> needs more
>>>>> code/testing:
>>>>>
>>>>> rtems_status_code
>>>>> rtems_clock_get_realtime(struct timespec *time_snapshot)
>>>>> {
>>>>>     if ( time_snapshot == NULL ) {
>>>>>       return RTEMS_INVALID_ADDRESS;
>>>>>     }
>>>>>
>>>>>     _Timecounter_Nanotime( time_snapshot );
>>>>>     return RTEMS_SUCCESSFUL;
>>>>> }
>>>>>
>>>>> 3. Return the structure and use the existing implementation:
>>>>>
>>>>> static inline struct timespec rtems_clock_get_realtime(void)
>>>>> {
>>>>>     struct timespec time_snapshot;
>>>>>
>>>>>     _Timecounter_Nanotime( &time_snapshot );
>>>>>
>>>>>     return time_snapshot;
>>>>> }
>>>>>
>>>>> This is currently not well supported by existing compilers:
>>>>>
>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103150
>>>>>
>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101926
>>>>>
>>>> 4. Do nothing for a NULL pointer:
>>>>
>>>> void rtems_clock_get_realtime(struct timespec *);
>>>>
>>>> This requires a wrapper function which can use a tail call 
>>>> optimization:
>>>>
>>>> void
>>>> rtems_clock_get_realtime(struct timespec *time_snapshot)
>>>> {
>>>>     if ( time_snapshot == NULL ) {
>>>>       return;
>>>>     }
>>>>
>>>>     _Timecounter_Nanotime( time_snapshot );
>>>> }
>>> How do we want to proceed with this? We ship the high performance and 
>>> very
>>> useful clock routines from FreeBSD since 2015. I just would like to 
>>> add an RTEMS
>>> signature for them and document them in the Clock Manager. Currently, 
>>> these
>>> routines are the most efficient way to get clock values in RTEMS. 
>>> Developers
>>> afraid of unchecked NULL pointers may use existing RTEMS directives or
>>> clock_get(). It would be nice if we can decide on a way forward.
>> I will leave this for Joel to decide.
> 
> Joel, what is your opinion on this? I accidentally committed this patch:
> 
> https://lists.rtems.org/pipermail/devel/2021-November/069984.html

I also checked in the documentation patch. We can adjust the API before 
the RTEMS 6 release if necessary.

https://docs.rtems.org/branches/master/c-user/clock/index.html

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/