| Message ID | 1438692658-22311-1-git-send-email-ross.lagerwall@citrix.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Aug 04, 2015 at 01:50:58PM +0100, Ross Lagerwall wrote: > Waking the dealloc thread before decrementing inflight_packets is racy > because it means the thread may go to sleep before inflight_packets is > decremented. If kthread_stop() has already been called, the dealloc > thread may wait forever with nothing to wake it. Instead, wake the > thread only after decrementing inflight_packets. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > --- > drivers/net/xen-netback/netback.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c > index 7d50711..e95ee20 100644 > --- a/drivers/net/xen-netback/netback.c > +++ b/drivers/net/xen-netback/netback.c > @@ -1536,7 +1536,6 @@ void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success) > smp_wmb(); > queue->dealloc_prod++; > } while (ubuf); > - wake_up(&queue->dealloc_wq); > spin_unlock_irqrestore(&queue->callback_lock, flags); > > if (likely(zerocopy_success)) > @@ -1544,6 +1543,7 @@ void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success) > else > queue->stats.tx_zerocopy_fail++; > xenvif_skb_zerocopy_complete(queue); > + wake_up(&queue->dealloc_wq); Can you move this wake_up into xenvif_skb_zerocopy_complete and have a comment there saying wake_up must be called after decrementing inflight counters (possibly with some of the commit message)? That way we don't trip over this in the future if we're to refactor the callback function. Wei. > } > > static inline void xenvif_tx_dealloc_action(struct xenvif_queue *queue) > -- > 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index 7d50711..e95ee20 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -1536,7 +1536,6 @@ void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success) smp_wmb(); queue->dealloc_prod++; } while (ubuf); - wake_up(&queue->dealloc_wq); spin_unlock_irqrestore(&queue->callback_lock, flags); if (likely(zerocopy_success)) @@ -1544,6 +1543,7 @@ void xenvif_zerocopy_callback(struct ubuf_info *ubuf, bool zerocopy_success) else queue->stats.tx_zerocopy_fail++; xenvif_skb_zerocopy_complete(queue); + wake_up(&queue->dealloc_wq); } static inline void xenvif_tx_dealloc_action(struct xenvif_queue *queue)
Waking the dealloc thread before decrementing inflight_packets is racy because it means the thread may go to sleep before inflight_packets is decremented. If kthread_stop() has already been called, the dealloc thread may wait forever with nothing to wake it. Instead, wake the thread only after decrementing inflight_packets. Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> --- drivers/net/xen-netback/netback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)