[Adeos-main] [BUG] ipipe_sync_stage deadlock

[Adeos-main] [BUG] ipipe_sync_stage deadlock

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]

I love KVM - not only because I'm heavily hacking on it now, also
because it is so easy to trigger various kinds of guest issues with
it (SMP host + threaded virtual CPUs => true SMP guest - and if you
want to watch fancy things happen, just load one host core).

Here is one I found that way:

void fastcall __ipipe_sync_stage(unsigned long syncmask)
{
[ ... ]
	while ((mask = (ipipe_this_cpudom_var(irqpend_himask) & syncmask)) != 0) {
		level = __ipipe_ffnz(mask);

		while ((submask = ipipe_this_cpudom_var(irqpend_lomask)[level]) != 0) {
			rank = __ipipe_ffnz(submask);
			irq = (level << IPIPE_IRQ_ISHIFT) + rank;

			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control)) {
				__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
				continue;
			}

			__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);

			if (ipipe_this_cpudom_var(irqpend_lomask)[level] == 0)
				__clear_bit(level, &ipipe_this_cpudom_var(irqpend_himask));
[ ... ]

Who sees the deadlock? 100 points for the right answer! But be quick,
I'm already hacking on a fix. :)

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [Adeos-main] [BUG] ipipe_sync_stage deadlock

[Philippe Gerum]

Jan Kiszka wrote:
> I love KVM - not only because I'm heavily hacking on it now, also
> because it is so easy to trigger various kinds of guest issues with
> it (SMP host + threaded virtual CPUs => true SMP guest - and if you
> want to watch fancy things happen, just load one host core).
> 
> Here is one I found that way:
> 
> void fastcall __ipipe_sync_stage(unsigned long syncmask)
> {
> [ ... ]
> 	while ((mask = (ipipe_this_cpudom_var(irqpend_himask) & syncmask)) != 0) {
> 		level = __ipipe_ffnz(mask);
> 
> 		while ((submask = ipipe_this_cpudom_var(irqpend_lomask)[level]) != 0) {
> 			rank = __ipipe_ffnz(submask);
> 			irq = (level << IPIPE_IRQ_ISHIFT) + rank;
> 
> 			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control)) {
> 				__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
- 				continue;
+				goto rpm_needs_more_neurons;
> 			}
> 
> 			__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
> 
+ rpm_needs_more_neurons:
> 			if (ipipe_this_cpudom_var(irqpend_lomask)[level] == 0)
> 				__clear_bit(level, &ipipe_this_cpudom_var(irqpend_himask));
> [ ... ]
> 
> Who sees the deadlock? 100 points for the right answer! But be quick,
> I'm already hacking on a fix. :)
> 
> Jan
> 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Adeos-main mailing list
> Adeos-main@domain.hid
> https://mail.gna.org/listinfo/adeos-main


-- 
Philippe.

Re: [Adeos-main] [BUG] ipipe_sync_stage deadlock

[Philippe Gerum]

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> I love KVM - not only because I'm heavily hacking on it now, also
>> because it is so easy to trigger various kinds of guest issues with
>> it (SMP host + threaded virtual CPUs => true SMP guest - and if you
>> want to watch fancy things happen, just load one host core).
>>
>> Here is one I found that way:
>>
>> void fastcall __ipipe_sync_stage(unsigned long syncmask)
>> {
>> [ ... ]
>> 	while ((mask = (ipipe_this_cpudom_var(irqpend_himask) & syncmask)) != 0) {
>> 		level = __ipipe_ffnz(mask);
>>
>> 		while ((submask = ipipe_this_cpudom_var(irqpend_lomask)[level]) != 0) {
>> 			rank = __ipipe_ffnz(submask);
>> 			irq = (level << IPIPE_IRQ_ISHIFT) + rank;
>>
>> 			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control)) {
>> 				__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
> - 				continue;
> +				goto rpm_needs_more_neurons;
>> 			}
>>
>> 			__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
>>
> + rpm_needs_more_neurons:
>> 			if (ipipe_this_cpudom_var(irqpend_lomask)[level] == 0)
>> 				__clear_bit(level, &ipipe_this_cpudom_var(irqpend_himask));
>> [ ... ]
>>

Of course, I'm not saying this is the real fix, but that just points at a
possible issue. I do want those 100 points on my score card.

>> Who sees the deadlock? 100 points for the right answer! But be quick,
>> I'm already hacking on a fix. :)
>>
>> Jan
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Adeos-main mailing list
>> Adeos-main@domain.hid
>> https://mail.gna.org/listinfo/adeos-main
> 
> 


-- 
Philippe.

Re: [Adeos-main] [BUG] ipipe_sync_stage deadlock

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]

Philippe Gerum wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> I love KVM - not only because I'm heavily hacking on it now, also
>>> because it is so easy to trigger various kinds of guest issues with
>>> it (SMP host + threaded virtual CPUs => true SMP guest - and if you
>>> want to watch fancy things happen, just load one host core).
>>>
>>> Here is one I found that way:
>>>
>>> void fastcall __ipipe_sync_stage(unsigned long syncmask)
>>> {
>>> [ ... ]
>>> 	while ((mask = (ipipe_this_cpudom_var(irqpend_himask) & syncmask)) != 0) {
>>> 		level = __ipipe_ffnz(mask);
>>>
>>> 		while ((submask = ipipe_this_cpudom_var(irqpend_lomask)[level]) != 0) {
>>> 			rank = __ipipe_ffnz(submask);
>>> 			irq = (level << IPIPE_IRQ_ISHIFT) + rank;
>>>
>>> 			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control)) {
>>> 				__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
>> - 				continue;
>> +				goto rpm_needs_more_neurons;
>>> 			}
>>>
>>> 			__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
>>>
>> + rpm_needs_more_neurons:
>>> 			if (ipipe_this_cpudom_var(irqpend_lomask)[level] == 0)
>>> 				__clear_bit(level, &ipipe_this_cpudom_var(irqpend_himask));
>>> [ ... ]
>>>
> 
> Of course, I'm not saying this is the real fix, but that just points at a
> possible issue. I do want those 100 points on my score card.

Well, I wasn't asking for a fix, just for spotting the gremlin - so you
won! :->

Then let us fix this for real:

---
 kernel/ipipe/core.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Index: b/kernel/ipipe/core.c
===================================================================
--- a/kernel/ipipe/core.c
+++ b/kernel/ipipe/core.c
@@ -957,16 +957,14 @@ void fastcall __ipipe_sync_stage(unsigne
 			rank = __ipipe_ffnz(submask);
 			irq = (level << IPIPE_IRQ_ISHIFT) + rank;
 
-			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control)) {
-				__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
-				continue;
-			}
-
 			__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
 
 			if (ipipe_this_cpudom_var(irqpend_lomask)[level] == 0)
 				__clear_bit(level, &ipipe_this_cpudom_var(irqpend_himask));
 
+			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control))
+				continue;
+
 			__set_bit(IPIPE_STALL_FLAG, &ipipe_this_cpudom_var(status));
 
 			if (ipd == ipipe_root_domain)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

Re: [Adeos-main] [BUG] ipipe_sync_stage deadlock

[Philippe Gerum]

Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Philippe Gerum wrote:
>>> Jan Kiszka wrote:
>>>> I love KVM - not only because I'm heavily hacking on it now, also
>>>> because it is so easy to trigger various kinds of guest issues with
>>>> it (SMP host + threaded virtual CPUs => true SMP guest - and if you
>>>> want to watch fancy things happen, just load one host core).
>>>>
>>>> Here is one I found that way:
>>>>
>>>> void fastcall __ipipe_sync_stage(unsigned long syncmask)
>>>> {
>>>> [ ... ]
>>>> 	while ((mask = (ipipe_this_cpudom_var(irqpend_himask) & syncmask)) != 0) {
>>>> 		level = __ipipe_ffnz(mask);
>>>>
>>>> 		while ((submask = ipipe_this_cpudom_var(irqpend_lomask)[level]) != 0) {
>>>> 			rank = __ipipe_ffnz(submask);
>>>> 			irq = (level << IPIPE_IRQ_ISHIFT) + rank;
>>>>
>>>> 			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control)) {
>>>> 				__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
>>> - 				continue;
>>> +				goto rpm_needs_more_neurons;
>>>> 			}
>>>>
>>>> 			__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
>>>>
>>> + rpm_needs_more_neurons:
>>>> 			if (ipipe_this_cpudom_var(irqpend_lomask)[level] == 0)
>>>> 				__clear_bit(level, &ipipe_this_cpudom_var(irqpend_himask));
>>>> [ ... ]
>>>>
>> Of course, I'm not saying this is the real fix, but that just points at a
>> possible issue. I do want those 100 points on my score card.
> 
> Well, I wasn't asking for a fix, just for spotting the gremlin - so you
> won! :->
> 

Ah! Great. So this is the ultimate proof that when it comes to software
development, one neuron is one too many!

> Then let us fix this for real:
>

Ok, will merge thanks.

> ---
>  kernel/ipipe/core.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> Index: b/kernel/ipipe/core.c
> ===================================================================
> --- a/kernel/ipipe/core.c
> +++ b/kernel/ipipe/core.c
> @@ -957,16 +957,14 @@ void fastcall __ipipe_sync_stage(unsigne
>  			rank = __ipipe_ffnz(submask);
>  			irq = (level << IPIPE_IRQ_ISHIFT) + rank;
>  
> -			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control)) {
> -				__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
> -				continue;
> -			}
> -
>  			__clear_bit(rank, &ipipe_this_cpudom_var(irqpend_lomask)[level]);
>  
>  			if (ipipe_this_cpudom_var(irqpend_lomask)[level] == 0)
>  				__clear_bit(level, &ipipe_this_cpudom_var(irqpend_himask));
>  
> +			if (test_bit(IPIPE_LOCK_FLAG, &ipd->irqs[irq].control))
> +				continue;
> +
>  			__set_bit(IPIPE_STALL_FLAG, &ipipe_this_cpudom_var(status));
>  
>  			if (ipd == ipipe_root_domain)
> 


-- 
Philippe.