This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, newlib] Allow locking routine to be retargeted


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!

> > 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);
}

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?

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]