diff options
author | sephe <sephe@FreeBSD.org> | 2016-02-29 03:54:51 +0000 |
---|---|---|
committer | sephe <sephe@FreeBSD.org> | 2016-02-29 03:54:51 +0000 |
commit | 0123218eb81ffcbe8643dfa06e722180f4a0b4f2 (patch) | |
tree | 77de2c9b3975e06502ae6969f455de2fcfce40d3 /sys/sys/buf_ring.h | |
parent | 8bf1194fe59ab6a4905efd0ff00cd6b7462247e1 (diff) | |
download | FreeBSD-src-0123218eb81ffcbe8643dfa06e722180f4a0b4f2.zip FreeBSD-src-0123218eb81ffcbe8643dfa06e722180f4a0b4f2.tar.gz |
buf_ring/drbr: Add buf_ring_peek_clear_sc and use it in drbr_peek
Unlike buf_ring_peek, it only supports single consumer mode, and it
clears the cons_head if DEBUG_BUFRING/INVARIANTS is defined.
The normal use case of drbr_peek for network drivers is:
m = drbr_peek(br);
err = hw_spec_encap(&m); /* could m_defrag/m_collapse */
(*)
if (err) {
if (m == NULL)
drbr_advance(br);
else
drbr_putback(br, m);
/* break the loop */
}
drbr_advance(br);
The race is:
If hw_spec_encap() m_defrag or m_collapse the mbuf, i.e. the old mbuf
was freed, or like the Hyper-V's network driver, that transmission-
done does not even require the TX lock; then on the other CPU at the
(*) time, the freed mbuf could be recycled and being drbr_enqueue even
before the current CPU had the chance to call drbr_{advance,putback}.
This triggers a panic in drbr_enqueue duplicated element check, if
DEBUG_BUFRING/INVARIANTS is defined.
Use buf_ring_peek_clear_sc() in drbr_peek() to fix the above race.
This change is a NO-OP, if neither DEBUG_BUFRING nor INVARIANTS are
defined.
MFC after: 1 week
Sponsored by: Microsoft OSTC
Differential Revision: https://reviews.freebsd.org/D5416
Diffstat (limited to 'sys/sys/buf_ring.h')
-rw-r--r-- | sys/sys/buf_ring.h | 31 |
1 files changed, 31 insertions, 0 deletions
diff --git a/sys/sys/buf_ring.h b/sys/sys/buf_ring.h index 012aabf..6a6bfc4 100644 --- a/sys/sys/buf_ring.h +++ b/sys/sys/buf_ring.h @@ -268,6 +268,37 @@ buf_ring_peek(struct buf_ring *br) 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) { |