[PATCH, newlib] Allow locking routine to be retargeted
Thomas Preudhomme
thomas.preudhomme@foss.arm.com
Fri Nov 11 11:09:00 GMT 2016
On 10/11/16 20:32, Freddie Chopin wrote:
> Hello again!
>
> On Thu, 2016-11-10 at 16:04 +0000, Thomas Preudhomme wrote:
>>> Why don't you do the same thing for recursive functions? At least
>>> the
>>> lock used by malloc() has to be recursive, so with your patch
>>> exactly
>>> all mutexes should be recursive too.
>>
>> Yes, I did not want to expose all functions until there is a need to
>> differentiate. it is easy to add them later but removing it if noone
>> use it
>> would be quite difficult.
>
> But there is a need to differentiate! Recursive lock is used 10 times
> in newlib (for example in malloc() / free()), non-recursive is used 4
> times (for example for locking time-zone info). If you only allow
> overriding the "standard" functions, you effectively force the user to
> use recursive locks everywhere. I'd definitely be "for" providing
> functions for both lock types!
My idea was indeed that all lock would have to be recursive in a first time and
differential later if that proves necessary. I'm happy to revisit that though.
>
>>> I have doubts about practical implementation of these functions for
>>> any
>>> RTOS, because of the __LOCK_INIT() macro used for initialization.
>>> In
>>> every retargeted function you'll have to start critical section
>>> (most
>>> likely by disabling interrupts) to initialize the object on heap,
>>> but
>>> then how would you use heap if malloc()'s lock is used via these
>>> functions too?
>>
>> I do not understand. Why would __LOCK_INIT need to start a critical
>> section?
>
> OK, this was not very clear. The problem with simple functions like the
> ones in your patch is that they are extremely hard to actually use in
> an RTOS. Or maybe it just seems so hard for me, that's also possible.
>
> This problem affects only "static" locks created with __LOCK_INIT and
> __LOCK_INIT_RECURSIVE macros. With your patch such locks are just
> uintptr_t with value 0. From my understanding it seems that such locks
> are considered to be "initialized". So you can call __lock_acquire() on
> such object right away. But how would such function look like?
>
> void __lock_acquire(_LOCK_T lock)
> {
> if (lock == 0)
> {
> // lock not yet really initialized...
> disableInterrupts();
> if (lock == 0) // re-check due to possible race condition
> lock = malloc(sizeof(RealMutexFromYourRtos));
>
> assert(lock != 0);
> rtos_mutex_create(lock);
> enableInterrupts();
> }
>
> rtos_mutex_lock((RealMutexFromYourRtos*)lock);
> }
Oh I see, that's very clear now, thank you.
>
> This could work, if only malloc() was not protected with the statically
> created _LOCK_T lock itself, which would need such initialization
> too... I don't see any other way to write such function for an RTOS
> which has mutexes with size greater than "uintptr_t" (probably huge
> majority). You could do it with a static pool of storage for mutexes,
> but then you'd have to know upfront how many you want and your RTOS
> would have to allow creation of locks in provided storage (most of them
> allow that, but not all). Maybe I just don't see the simplest solution
> to this problem?
Why do mutex needs to be more than a single integer? How big are mutex in RTOS
typically in your experience?
>
> Don't get me wrong - I'd really like something like this to be in
> newlib. It just seems that this is extremely hard due to the
> __LOCK_INIT macro which hardcodes the actual size of storage... I'm
> really interested in that feature, as I'd like to provide better newlib
> integration for the RTOS I'm writing ( http://distortos.org/ ).
>
> Regasrds,
> FCh
>
Best regards,
Thomas
More information about the Newlib
mailing list