[Cbe-oss-dev] [PATCH 2/3] spufs context switch - make SPU_CONTEXT_SWITCH_PENDING atomic

[Luke Browning]

On Thu, 2008-04-24 at 08:09 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2008-04-23 at 13:56 -0300, Luke Browning wrote:
> > 
> > Do you agree that there is still an error?  The flag needs to be set
> > before, not after the synchronize routine.
> 
> Yes, however, there are other issues since we need to actually be able
> to take interrupts while in the context switch to service faults on the
> kernel mapping.
> 
> > > - The flag is unnecessary alltogether I believe because we mask all
> > the
> > > interrupts on the SPU before calling synchronize_irq and our
> > interrupt
> > > handler checks for the mask before processing anything.
> > 
> > What if the exception occurs before the context switch and is handed
> > off
> > to the controlling thread.  The controlling thread uses the values in
> > the csa, not the hardware registers, so I think you still need the
> > flag
> > and the explicit locking in my patch as the controlling thread runs
> > completely independently.  
> 
> I don't see why. We have cleared the condition in the HW (ie. DSISR)
> when copying to the CSA. I can understand that there is some uncertainty
> on when the HW may re-emit the interrupt especially for segment faults,
> and we are trying to sort that out, but as I said, we -do- need to be
> able to handle those class 1 interrupts for the context switch code
> itself anyway, so we need to sort out a different problem.
> 
> > The CS pending flag is set and reset multiple times within the context
> > switch algorithm.  I extended the use of this flag to cover any place
> > where the CS algorithm is dealing with the MFC queue and I didn't want
> > the controlling thread to restart the dma.  
> 
> But the controlling thread wouldn't, because of the context mutex, no ?
> It can only restart the DMA if it can take that mutex (which it can't
> when context switching out) and the context is still physically in the
> SPE. If the controlling thread does the DMA restart without the context
> mutex, then that is a bug that needs fixing.

yes, that makes sense.  I guess there are just the two bugs:

1) setting the PENDING CS flag before the irq_synchronize() call.  
2) fixing the pending interrupt bug on the kernel mapping.

Luke