[Differential] [Closed] D5416: buf_ring/drbr: Add buf_ring_peek_clear_sc and use it in drbr_peek

[Phabricator]

This revision was automatically updated to reflect the committed changes.
Closed by commit rS296178: buf_ring/drbr: Add buf_ring_peek_clear_sc and use it 
in drbr_peek (authored by sephe).

CHANGED PRIOR TO COMMIT
  https://reviews.freebsd.org/D5416?vs=13669&id=13866#toc

REPOSITORY
  rS FreeBSD src repository

CHANGES SINCE LAST UPDATE
  https://reviews.freebsd.org/D5416?vs=13669&id=13866

REVISION DETAIL
  https://reviews.freebsd.org/D5416

AFFECTED FILES
  head/sys/net/ifq.h
  head/sys/sys/buf_ring.h

CHANGE DETAILS
  diff --git a/head/sys/sys/buf_ring.h b/head/sys/sys/buf_ring.h
  --- a/head/sys/sys/buf_ring.h
  +++ b/head/sys/sys/buf_ring.h
  @@ -268,6 +268,37 @@
        return (br->br_ring[br->br_cons_head]);
   }
   
  +static __inline void *
  +buf_ring_peek_clear_sc(struct buf_ring *br)
  +{
  +#ifdef DEBUG_BUFRING
  +     void *ret;
  +
  +     if (!mtx_owned(br->br_lock))
  +             panic("lock not held on single consumer dequeue");
  +#endif       
  +     /*
  +      * I believe it is safe to not have a memory barrier
  +      * here because we control cons and tail is worst case
  +      * a lagging indicator so we worst case we might
  +      * return NULL immediately after a buffer has been enqueued
  +      */
  +     if (br->br_cons_head == br->br_prod_tail)
  +             return (NULL);
  +
  +#ifdef DEBUG_BUFRING
  +     /*
  +      * Single consumer, i.e. cons_head will not move while we are
  +      * running, so atomic_swap_ptr() is not necessary here.
  +      */
  +     ret = br->br_ring[br->br_cons_head];
  +     br->br_ring[br->br_cons_head] = NULL;
  +     return (ret);
  +#else
  +     return (br->br_ring[br->br_cons_head]);
  +#endif
  +}
  +
   static __inline int
   buf_ring_full(struct buf_ring *br)
   {
  diff --git a/head/sys/net/ifq.h b/head/sys/net/ifq.h
  --- a/head/sys/net/ifq.h
  +++ b/head/sys/net/ifq.h
  @@ -369,7 +369,7 @@
                return (m);
        }
   #endif
  -     return(buf_ring_peek(br));
  +     return(buf_ring_peek_clear_sc(br));
   }
   
   static __inline void

EMAIL PREFERENCES
  https://reviews.freebsd.org/settings/panel/emailpreferences/

To: sepherosa_gmail.com, network, adrian, delphij, rrs, glebius, kmacy, 
decui_microsoft.com, honzhan_microsoft.com, howard0su_gmail.com
Cc: freebsd-net-list

diff --git a/head/sys/sys/buf_ring.h b/head/sys/sys/buf_ring.h
--- a/head/sys/sys/buf_ring.h
+++ b/head/sys/sys/buf_ring.h
@@ -268,6 +268,37 @@
 	return (br->br_ring[br->br_cons_head]);
 }
 
+static __inline void *
+buf_ring_peek_clear_sc(struct buf_ring *br)
+{
+#ifdef DEBUG_BUFRING
+	void *ret;
+
+	if (!mtx_owned(br->br_lock))
+		panic("lock not held on single consumer dequeue");
+#endif	
+	/*
+	 * I believe it is safe to not have a memory barrier
+	 * here because we control cons and tail is worst case
+	 * a lagging indicator so we worst case we might
+	 * return NULL immediately after a buffer has been enqueued
+	 */
+	if (br->br_cons_head == br->br_prod_tail)
+		return (NULL);
+
+#ifdef DEBUG_BUFRING
+	/*
+	 * Single consumer, i.e. cons_head will not move while we are
+	 * running, so atomic_swap_ptr() is not necessary here.
+	 */
+	ret = br->br_ring[br->br_cons_head];
+	br->br_ring[br->br_cons_head] = NULL;
+	return (ret);
+#else
+	return (br->br_ring[br->br_cons_head]);
+#endif
+}
+
 static __inline int
 buf_ring_full(struct buf_ring *br)
 {
diff --git a/head/sys/net/ifq.h b/head/sys/net/ifq.h
--- a/head/sys/net/ifq.h
+++ b/head/sys/net/ifq.h
@@ -369,7 +369,7 @@
 		return (m);
 	}
 #endif
-	return(buf_ring_peek(br));
+	return(buf_ring_peek_clear_sc(br));
 }
 
 static __inline void


_______________________________________________
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"