[parisc-linux] Mysterious hangs with parisc (a send_group_sig_info() analysis)

[Michael S. Zick]

On Sat July 22 2006 17:10, Joel Soete wrote:
> 
> Carlos O'Donell wrote:
> >>         Q4: imho the most important: with 2 gcc where is the couple of 
> >> ssm/rsm that jejb introduced in his patch:
> > 
> > 
> > The *most* important piece of imformation is the pre-compiled source.
>     if (flags & 0x00000001) {
>      __asm__ __volatile__("ssm %0,%%r0\n" : : "i" (0x00000001) : "memory" );
>      __asm__ __volatile__("":::"memory");
>      __asm__ __volatile__("rsm %0,%%r0\n" : : "i" (0x00000001) : "memory" );
>     } else
>      __asm__ __volatile__("":::"memory");
> 
> > You *must* determine that both paths of the if/else are different,
> > because if they are the same, then GCC is right to remove one of the
> > paths, and unconditionally call cpu_relax.
> > 

Note: That code is the same when either compiler is used;
_But_ as a separate problem...

My reading of the intent of that added code was:

Busy_Loop:
   If (Interrupts are disabled) {
      Enable interrupts;
      Cpu_relax; /* Long enough to recognize pending interrupts */
      Disable interrupts;
   } else {
      Cpu_relax;
   }

And my complaint was a matter of structure...

The assumption here is that the flag bit in an external variable
is always in-sync with the interrupt status bit in the register.

Obviously a false assumption, this routine is an example...

Inside the first if block, the status bit is changed but the external
variable bit is not.

A general purpose, like we don't know what state the interrupts
are in, and we don't want to trust an external flag bit;

Looks like:

Busy_Loop:
   Save current interrupt status to the external variable;
   Enable interrupts; /* a nop if already enabled */
   Cpu_relax; /* Long enough to recognize pending interrupts */
   Restore interrupt status from external variable;

Our redefining Cpu_relax to a pair of nops was for machines that
delay recognizing interrupts for an instruction after changing
the status bit.  We also included an asm comment in Cpu_relax
just in case we wanted to see that it had not gotten optimized away.

Same reason the mb() was redefined as an asm comment that could
be identified by inspection.

Mike