Project Home
Project Home
Documents
Documents
Wiki
Wiki
Discussion Forums
Discussions
Project Information
Project Info
Forum Topic - Priority inversion in iofunc_attr_lock: (11 Items)
   
Priority inversion in iofunc_attr_lock  
Hi,

Why was _sleepon_wait() used in int iofunc_attr_lock(iofunc_attr_t *attr)? It's an obvious priority inversion and the 
risk of it is created on every file open request. Why was not to implement the same functionality with a simple 
recursive mutex with priority inheritance?

The same approach with sleepons is used in _iofunc_llist_lock().
Re: Priority inversion in iofunc_attr_lock  
Guys,

Have have the courage to acknowledge your bugs and create a PR at least (I'm not so naive to be dreaming about an urgent
 update for all the OS releases) if you have bravery to use "Realtime" word within the name of your product.
RE: Priority inversion in iofunc_attr_lock  
Seems we got ourselves a skilled motivator.

> -----Original Message-----
> From: Oleh Derevenko [mailto:community-noreply@qnx.com]
> Sent: January-22-09 9:25 AM
> To: ostech-core_os
> Subject: Re: Priority inversion in iofunc_attr_lock
> 
> Guys,
> 
> Have have the courage to acknowledge your bugs and create a PR at least
> (I'm not so naive to be dreaming about an urgent update for all the OS
> releases) if you have bravery to use "Realtime" word within the name of
> your product.
> 
> _______________________________________________
> OSTech
> http://community.qnx.com/sf/go/post20579
> 
Re: Priority inversion in iofunc_attr_lock  
Thanks for the helpful comment.  It's already been raised as PR48722.

Oleh Derevenko wrote:
> Hi,
> 
> Why was _sleepon_wait() used in int iofunc_attr_lock(iofunc_attr_t *attr)? It's an obvious priority inversion and the 
risk of it is created on every file open request. Why was not to implement the same functionality with a simple 
recursive mutex with priority inheritance?
> 
> The same approach with sleepons is used in _iofunc_llist_lock().
> 
> 
> _______________________________________________
> OSTech
> http://community.qnx.com/sf/go/post20421
> 

-- 
cburgess@qnx.com
Re: Priority inversion in iofunc_attr_lock  
Hi Colin,

Great to find out! I hope you'll find one more pthread_cond_wait() in _resmgr_handle.c without my assistance. ;)

> Thanks for the helpful comment.  It's already been raised as PR48722.
> 
RE: Priority inversion in iofunc_attr_lock  
I miss Igor.

> -----Original Message-----
> From: Oleh Derevenko [mailto:community-noreply@qnx.com]
> Sent: January-23-09 11:17 AM
> To: ostech-core_os
> Subject: Re: Priority inversion in iofunc_attr_lock
> 
> Hi Colin,
> 
> Great to find out! I hope you'll find one more pthread_cond_wait() in
> _resmgr_handle.c without my assistance. ;)
> 
> > Thanks for the helpful comment.  It's already been raised as PR48722.
> >
> 
> 
> _______________________________________________
> OSTech
> http://community.qnx.com/sf/go/post20714
> 
Re: Priority inversion in iofunc_attr_lock  
Sometimes we do too... it's been a while since we've been "Igored" :-)

Just on a personal note, I love passion in users, and it's great when
problems are brought to our attention.  But please remember that
we (techies) are NOT product management, and we don't generally get the
final word on what is worked on and when.   :-)

Colin

Mario Charest wrote:
> I miss Igor.
> 
>> -----Original Message-----
>> From: Oleh Derevenko [mailto:community-noreply@qnx.com]
>> Sent: January-23-09 11:17 AM
>> To: ostech-core_os
>> Subject: Re: Priority inversion in iofunc_attr_lock
>>
>> Hi Colin,
>>
>> Great to find out! I hope you'll find one more pthread_cond_wait() in
>> _resmgr_handle.c without my assistance. ;)
>>
>>> Thanks for the helpful comment.  It's already been raised as PR48722.
>>>
>>
>> _______________________________________________
>> OSTech
>> http://community.qnx.com/sf/go/post20714
>>
> 
> 
> _______________________________________________
> OSTech
> http://community.qnx.com/sf/go/post20715
> 

-- 
cburgess@qnx.com
Re: Priority inversion in iofunc_attr_lock  
> Just on a personal note, I love passion in users, and it's great when
> problems are brought to our attention.  But please remember that
> we (techies) are NOT product management, and we don't generally get the
> final word on what is worked on and when.   :-)

But you should have influence on product management and be able to raise bug priorities. Managers are aware of product 
development plan and new features that are to be added but usually they are unable to evaluate themselves the severity/
impact of a given bug. The developers are the "final authority" in bug evaluation should have the right to request the 
issue to be fixed in first turn if they find that the impact on the product functionality/reliability is severe.
For example we have the rule of fixing bugs on high priority on our project. If I find a bug or QA team reports me a bug
 and I usually stop ongoing development and fix the bug because that's more important. There are rare exceptions in 
cases if it's necessary to implement some feature/fix for the customers on demand in a day or two or if there is no good
 way of fixing the problem with relatively small effort at that time.
Though I understand that every company and even every project within a company can have its own rules.
Re: Priority inversion in iofunc_attr_lock  
Hi Oleh,

Yeah, when this PR is addressed we will most certainly address these issues.

I apologize for my previously short reply - the OS group WAS were discussing it, and sometimes we forget that
you can't read our minds. :-)

I'll try and remember to update you when we have a timeline for fixing it.

Regards,

Colin

Oleh Derevenko wrote:
> Hi Colin,
> 
> Great to find out! I hope you'll find one more pthread_cond_wait() in _resmgr_handle.c without my assistance. ;)
> 
>> Thanks for the helpful comment.  It's already been raised as PR48722.
>>
> 
> 
> _______________________________________________
> OSTech
> http://community.qnx.com/sf/go/post20714
> 

-- 
cburgess@qnx.com
Re: Priority inversion in iofunc_attr_lock  
Oleh Derevenko wrote:
>  Hi,
>
>  Why was _sleepon_wait() used in int iofunc_attr_lock(iofunc_attr_t
>  *attr)? It's an obvious priority inversion and the risk of it is
>  created on every file open request. Why was not to implement the same
>  functionality with a simple recursive mutex with priority
>  inheritance?
>
>  The same approach with sleepons is used in _iofunc_llist_lock().


At the risk of further fanning the flames on this topic ... the 
replacement is not
quite as simple as a swap of one for the other since mutex's and sleepon's
are different things all together.

Note that a sleepon is just a cover for a condvar which _also_ is not 
priority
inheriting.  A mutex is great if you have a area you want to block on and a
_short_ amount of work you want to do, but if you want to wait for 
notification
of a more complex activity that doesn't need to be serialized while work is
going on then a condvar is typically used.  You need to build the priority
inheritance for those things into what ever work path is being handled 
... which
is awkward to do in user space without resorting to manual priority bumping
which is very error prone.

Just as a note, there was some work done several years ago to make all of
the synchronization primitives at least capable (with some user assistance)
of appropriate priority inheritance.  Don't know what became of it, but the
code is floating around.

Thomas
Re: Priority inversion in iofunc_attr_lock  
I know the cases when the mutex can be used and when condvar is a choice. Here with attribute locking the sleepon is 
used from single thread and is used in a manner the mutexes usually are. The only benefit of sleepon here is that you do
 not need to create a custom mutex pool - it's already embedded inside of sleepon.
If I make a patch to replace this (and maybe other condvars in resource manager) and you consider it to be good, would 
you merge it into your SVN?

To show you that mutex is quite a powerful tool I can propose you all guys to think on the following problem.

Consider two threads: a supplier and a consumer which transfer the sequence of data elements via a shared variable. I'll
 write the pseudocode in terms of Windows events (consider it to incorporate a condvar with a dedicated mutex and all 
the necessary signal/error handling).
The typical scheme for this communication is al follows.

EVENT data_ready; // Initially nonsignalled
EVENT data_extracted; // Initially signalled
void *shared_var;

/* PROVIDER THREAD */

while (1)
{
  void *next_value = OBTAIN_INPUT();

  WAIT_EVENT(data_extracted);
  shared_var = next_value;
  SIGNAL_EVENT(data_ready);
}

/* CONSUMER THREAD */

while (1)
{
  void *next_value;

  WAIT_EVENT(data_ready);
  next_value = shared_var;
  SIGNAL_EVENT(data_extracted);

  PROCESS_INPUT(next_value);
}

Now try to implement this scheme with the aid of mutexes only, without use of condvars/events. ;) It's possible.