svn commit: r309372 - head/sys/sys

[Sepherosa Ziehau]

peek_clear_sc is added to address the issue you mentioned.  IMHO, this
commit weakens the proper assertion.

On Fri, Dec 2, 2016 at 5:08 AM, Ryan Stone <rstone at freebsd.org> wrote:
> Author: rstone
> Date: Thu Dec  1 21:08:42 2016
> New Revision: 309372
> URL: https://svnweb.freebsd.org/changeset/base/309372
>
> Log:
>   Fix a false positive in a buf_ring assert
>
>   buf_ring contains an assert that checks whether an item being
>   enqueued already exists on the ring.  There is a subtle bug in
>   this assert.  An item can be returned by a peek() function and
>   freed, and then the consumer thread can be preempted before
>   calling advance().  If this happens the item appears to still be
>   on the queue, but another thread may allocate the item from the
>   free pool and wind up trying to enqueue it again, causing the
>   assert to trigger incorrectly.
>
>   Fix this by skipping the head of the consumer's portion of the
>   ring, as this index is what will be returned by peek().
>
>   Sponsored by: Dell EMC Isilon
>   MFC After:    1 week
>   Differential Revision:        https://reviews.freebsd.org/D8685
>   Reviewed by:  hselasky
>
> Modified:
>   head/sys/sys/buf_ring.h
>
> Modified: head/sys/sys/buf_ring.h
> ==============================================================================
> --- head/sys/sys/buf_ring.h     Thu Dec  1 20:36:48 2016        (r309371)
> +++ head/sys/sys/buf_ring.h     Thu Dec  1 21:08:42 2016        (r309372)
> @@ -67,11 +67,13 @@ buf_ring_enqueue(struct buf_ring *br, vo
>         uint32_t prod_head, prod_next, cons_tail;
>  #ifdef DEBUG_BUFRING
>         int i;
> -       for (i = br->br_cons_head; i != br->br_prod_head;
> -            i = ((i + 1) & br->br_cons_mask))
> -               if(br->br_ring[i] == buf)
> -                       panic("buf=%p already enqueue at %d prod=%d cons=%d",
> -                           buf, i, br->br_prod_tail, br->br_cons_tail);
> +       if (br->br_cons_head != br->br_prod_head) {
> +               for (i = (br->br_cons_head + 1) & br->br_cons_mask; i != br->br_prod_head;
> +                   i = ((i + 1) & br->br_cons_mask))
> +                       if(br->br_ring[i] == buf)
> +                               panic("buf=%p already enqueue at %d prod=%d cons=%d",
> +                                   buf, i, br->br_prod_tail, br->br_cons_tail);
> +       }
>  #endif
>         critical_enter();
>         do {
> _______________________________________________
> svn-src-all at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"



-- 
Tomorrow Will Never Die