On Sat June 12 2010 01:16:19 you wrote:
> What's the purpose of ath5k_rx_stop setting sc->rxlink to NULL?

good question. it has the comment /* just in case */ and this is in the HAL 
too, so i guess it was just copied here "just in case" and because nobody is 
sure we dont need it.

> As Bob points out, it causes problems if it's not done while holding the
> rx buffer spinlock.
> 
> There's code in ath5k_intr that sets sc->rxlink to NULL, apparently
> to handle a chip bug.  It needs to hold the spinlock as well to avoid
> self-linked buffers in the middle of the rx buffer list.

yes, i agree. we need to hold the spinlock while changing rxlink!

> I'm not sure this patch fixes anything, however.  If reading the RXDP
> register is not reliable, the driver already won't work correctly.

that's true. it's yet another "just in case"... so let's forget this patch.

bob (robert brown), did you get around to test my patch disabling the 
tasklets? i'm quite optimistic it could solve the problem, even though it 
might not be the final solution. if it helps we know this is the right 
track...

bruno

> bob
> 
> ====================
> 
> On Fri, Jun 11, 2010 at 11:14 AM, Bob Copeland <m...@bobcopeland.com> wrote:
> > On Fri, Jun 11, 2010 at 5:50 AM, Bruno Randolf <b...@einfach.org> wrote:
> > > Add an extra check to be sure we never process the self-linked rx
> > 
> > descriptor
> > 
> > > at the end of the list. This should not happen since in this case the
> > 
> > RXDP
> > 
> > Here's a way this might happen frequently BTW:
> > 
> > cpu 0:                                      cpu 1:
> > ath5k_rx_stop
> > 
> >                                            ath5k_tasklet_rx
> > 
> > // following not protected by rxbuf lock
> > sc->rxlink = NULL;   /* just in case */
> > 
> >                                              // following doesn't link
> >                                              used // buffer to prev.
> >                                              ath5k_rxbuf_setup()
> > 
> > AFAICT this can lead to lots of self-linked descriptors even in the
> > middle of the list.
> > 
> > --
> > Bob Copeland %% www.bobcopeland.com
> > _______________________________________________
> > ath5k-devel mailing list
> > ath5k-devel@lists.ath5k.org
> > https://lists.ath5k.org/mailman/listinfo/ath5k-devel
_______________________________________________
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

Reply via email to