RE: [PATCH v3 06/10] net/macb: clean up ring buffer logic
> return (TX_RING_SIZE - (bp->tx_head - bp->tx_tail) & (TX_RING_SIZE - > 1)); Is equivalent to: return (bp->tx_tail - bp->tx_head) & (TX_RING_SIZE - 1)); David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 06/10] net/macb: clean up ring buffer logic
On 10/31/2012 10:59 AM, Nicolas Ferre : > On 10/30/2012 07:22 PM, Håvard Skinnemoen : >> On Tue, Oct 30, 2012 at 4:12 AM, David Laight >> wrote: Instead of masking head and tail every time we increment them, just let them wrap through UINT_MAX and mask them when subscripting. Add simple accessor functions to do the subscripting properly to minimize the chances of messing this up. >>> ... +static unsigned int macb_tx_ring_avail(struct macb *bp) +{ + return TX_RING_SIZE - (bp->tx_head - bp->tx_tail); +} >>> >>> That one doesn't look quite right to me. >>> Surely it should be masking with 'TX_RING_SIZE - 1' >> >> Why is that? head and tail can never be more than TX_RING_SIZE apart, >> so it shouldn't make any difference. > > Absolutely. Well not so absolute, after having thinking twice ;-) We should move to: static unsigned int macb_tx_ring_avail(struct macb *bp) { return (TX_RING_SIZE - (bp->tx_head - bp->tx_tail) & (TX_RING_SIZE - 1)); } Thanks David! (sorry for the noise) Bye, -- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 06/10] net/macb: clean up ring buffer logic
> On Tue, Oct 30, 2012 at 4:12 AM, David Laight wrote: > >> Instead of masking head and tail every time we increment them, just let > >> them > >> wrap through UINT_MAX and mask them when subscripting. Add simple accessor > >> functions to do the subscripting properly to minimize the chances of > >> messing > >> this up. > > ... > >> +static unsigned int macb_tx_ring_avail(struct macb *bp) > >> +{ > >> + return TX_RING_SIZE - (bp->tx_head - bp->tx_tail); > >> +} > > > > That one doesn't look quite right to me. > > Surely it should be masking with 'TX_RING_SIZE - 1' > > Why is that? head and tail can never be more than TX_RING_SIZE apart, > so it shouldn't make any difference. It's a ring buffer (I presume) the pointers can be in either order. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 06/10] net/macb: clean up ring buffer logic
On 10/30/2012 07:22 PM, Håvard Skinnemoen : > On Tue, Oct 30, 2012 at 4:12 AM, David Laight wrote: >>> Instead of masking head and tail every time we increment them, just let them >>> wrap through UINT_MAX and mask them when subscripting. Add simple accessor >>> functions to do the subscripting properly to minimize the chances of messing >>> this up. >> ... >>> +static unsigned int macb_tx_ring_avail(struct macb *bp) >>> +{ >>> + return TX_RING_SIZE - (bp->tx_head - bp->tx_tail); >>> +} >> >> That one doesn't look quite right to me. >> Surely it should be masking with 'TX_RING_SIZE - 1' > > Why is that? head and tail can never be more than TX_RING_SIZE apart, > so it shouldn't make any difference. Absolutely. Best regards, -- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 06/10] net/macb: clean up ring buffer logic
On 10/30/2012 07:22 PM, Håvard Skinnemoen : On Tue, Oct 30, 2012 at 4:12 AM, David Laight david.lai...@aculab.com wrote: Instead of masking head and tail every time we increment them, just let them wrap through UINT_MAX and mask them when subscripting. Add simple accessor functions to do the subscripting properly to minimize the chances of messing this up. ... +static unsigned int macb_tx_ring_avail(struct macb *bp) +{ + return TX_RING_SIZE - (bp-tx_head - bp-tx_tail); +} That one doesn't look quite right to me. Surely it should be masking with 'TX_RING_SIZE - 1' Why is that? head and tail can never be more than TX_RING_SIZE apart, so it shouldn't make any difference. Absolutely. Best regards, -- Nicolas Ferre -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 06/10] net/macb: clean up ring buffer logic
On Tue, Oct 30, 2012 at 4:12 AM, David Laight david.lai...@aculab.com wrote: Instead of masking head and tail every time we increment them, just let them wrap through UINT_MAX and mask them when subscripting. Add simple accessor functions to do the subscripting properly to minimize the chances of messing this up. ... +static unsigned int macb_tx_ring_avail(struct macb *bp) +{ + return TX_RING_SIZE - (bp-tx_head - bp-tx_tail); +} That one doesn't look quite right to me. Surely it should be masking with 'TX_RING_SIZE - 1' Why is that? head and tail can never be more than TX_RING_SIZE apart, so it shouldn't make any difference. It's a ring buffer (I presume) the pointers can be in either order. David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 06/10] net/macb: clean up ring buffer logic
On 10/31/2012 10:59 AM, Nicolas Ferre : On 10/30/2012 07:22 PM, Håvard Skinnemoen : On Tue, Oct 30, 2012 at 4:12 AM, David Laight david.lai...@aculab.com wrote: Instead of masking head and tail every time we increment them, just let them wrap through UINT_MAX and mask them when subscripting. Add simple accessor functions to do the subscripting properly to minimize the chances of messing this up. ... +static unsigned int macb_tx_ring_avail(struct macb *bp) +{ + return TX_RING_SIZE - (bp-tx_head - bp-tx_tail); +} That one doesn't look quite right to me. Surely it should be masking with 'TX_RING_SIZE - 1' Why is that? head and tail can never be more than TX_RING_SIZE apart, so it shouldn't make any difference. Absolutely. Well not so absolute, after having thinking twice ;-) We should move to: static unsigned int macb_tx_ring_avail(struct macb *bp) { return (TX_RING_SIZE - (bp-tx_head - bp-tx_tail) (TX_RING_SIZE - 1)); } Thanks David! (sorry for the noise) Bye, -- Nicolas Ferre -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 06/10] net/macb: clean up ring buffer logic
return (TX_RING_SIZE - (bp-tx_head - bp-tx_tail) (TX_RING_SIZE - 1)); Is equivalent to: return (bp-tx_tail - bp-tx_head) (TX_RING_SIZE - 1)); David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 06/10] net/macb: clean up ring buffer logic
On Tue, Oct 30, 2012 at 4:12 AM, David Laight wrote: >> Instead of masking head and tail every time we increment them, just let them >> wrap through UINT_MAX and mask them when subscripting. Add simple accessor >> functions to do the subscripting properly to minimize the chances of messing >> this up. > ... >> +static unsigned int macb_tx_ring_avail(struct macb *bp) >> +{ >> + return TX_RING_SIZE - (bp->tx_head - bp->tx_tail); >> +} > > That one doesn't look quite right to me. > Surely it should be masking with 'TX_RING_SIZE - 1' Why is that? head and tail can never be more than TX_RING_SIZE apart, so it shouldn't make any difference. Havard -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 06/10] net/macb: clean up ring buffer logic
> Instead of masking head and tail every time we increment them, just let them > wrap through UINT_MAX and mask them when subscripting. Add simple accessor > functions to do the subscripting properly to minimize the chances of messing > this up. ... > +static unsigned int macb_tx_ring_avail(struct macb *bp) > +{ > + return TX_RING_SIZE - (bp->tx_head - bp->tx_tail); > +} That one doesn't look quite right to me. Surely it should be masking with 'TX_RING_SIZE - 1' David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 06/10] net/macb: clean up ring buffer logic
Instead of masking head and tail every time we increment them, just let them wrap through UINT_MAX and mask them when subscripting. Add simple accessor functions to do the subscripting properly to minimize the chances of messing this up. ... +static unsigned int macb_tx_ring_avail(struct macb *bp) +{ + return TX_RING_SIZE - (bp-tx_head - bp-tx_tail); +} That one doesn't look quite right to me. Surely it should be masking with 'TX_RING_SIZE - 1' David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 06/10] net/macb: clean up ring buffer logic
On Tue, Oct 30, 2012 at 4:12 AM, David Laight david.lai...@aculab.com wrote: Instead of masking head and tail every time we increment them, just let them wrap through UINT_MAX and mask them when subscripting. Add simple accessor functions to do the subscripting properly to minimize the chances of messing this up. ... +static unsigned int macb_tx_ring_avail(struct macb *bp) +{ + return TX_RING_SIZE - (bp-tx_head - bp-tx_tail); +} That one doesn't look quite right to me. Surely it should be masking with 'TX_RING_SIZE - 1' Why is that? head and tail can never be more than TX_RING_SIZE apart, so it shouldn't make any difference. Havard -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/