Re: [Xenomai] [PATCH v3] nucleus: Convert intrlock to a sleeping Linux lock

[Jan Kiszka <jan.kiszka@web.de>]

On 2013-01-06 15:26, Gilles Chanteperdrix wrote:
> On 01/06/2013 03:20 PM, Jan Kiszka wrote:
> 
>> On 2013-01-06 14:55, Gilles Chanteperdrix wrote:
>>> On 01/06/2013 12:03 PM, Jan Kiszka wrote:
>>>
>>>> On 2013-01-06 11:55, Jan Kiszka wrote:
>>>>> On 2013-01-06 11:14, Philippe Gerum wrote:
>>>>>> On 01/03/2013 03:09 PM, Jan Kiszka wrote:
>>>>>>
>>>>>>> +static struct xnvfile_lock_ops vfile_stat_lockops = {
>>>>>>> +	.get = xnintr_get_query_lock,
>>>>>>> +	.put = xnintr_put_query_lock,
>>>>>>> +};
>>>>>>> +
>>>>>>>  static struct xnvfile_snapshot stat_vfile = {
>>>>>>>  	.privsz = sizeof(struct vfile_stat_priv),
>>>>>>>  	.datasz = sizeof(struct vfile_stat_data),
>>>>>>>  	.tag = &nkpod_struct.threadlist_tag,
>>>>>>>  	.ops = &vfile_stat_ops,
>>>>>>> +	.entry = { .lockops = &vfile_stat_lockops },
>>>>>>>  };
>>>>>>
>>>>>> This would introduce a regression. stat_vfile's rewind/next handlers
>>>>>> compete with code altering the global thread queue which may run in
>>>>>> primary mode, such as xnpod_delete_thread() when deleting a kernel task.
>>>>>> So we can't use a linux lock to protect it.
>>>>>
>>>>> The Linux lock is held around this rewind loop (which is still in
>>>>> place), and it is not protecting anything thread related, just the
>>>>> appearance and disappearance of IRQs. I fail to see a conflict, and
>>>>> there are also no I-pipe/Xenomai warnings triggered with this pattern.
>>>>
>>>> Oh, now I see: The nklock is automatically taken if there are no
>>>> lockops. That needs to be done in the stat lockups as well, of course.
>>>> Will fix.
>>>
>>>
>>> The more I think about this, the more I find we should not be doing
>>> this. Xenomai 2.6.x is a stable branch, let us stop making such changes,
>>> this will allow us to start working on -forge. The warning is
>>> essentially a false-positive, I do not see any problem calling
>>> ipipe_virtualize_irq with the root domain stalled. We can skip it in the
>>> I-pipe core patches when compiling with CONFIG_IPIPE_LEGACY.
>>
>> Well, we need to support 2.6 for quite a while here, so I'm interested
>> in fixing such regressions,
> 
> 
> I repeat again: there is no regression, it is not invalid to call
> ipipe_virtualize_irq with the root domain stalled, and never was, the
> warning is new and a false positive. I am opposed to changing intrlock
> into a mutex with a lot of cascading consequences, the ones we see, the
> ones we will discover when people start using the next release.
> 
> If you want to benefit from the full instrumentation of the I-pipe core,
> use xenomai-forge.

Unfortunately, forge wasn't ready for our workload last time we checked.
That's why we are stabilizing I-pipe x86 over 2.6 here, and that's why
we need working instrumentations also in legacy mode.

If you prefer to skip the warning in ipipe, then I will send a
corresponding patch. The lock conversion is still necessary for forge,
though.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20130106/efc4a149/attachment.pgp>