Re: [Xen-devel] [PATCH] xen/netback: Wake dealloc thread after completing zerocopy work

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
>  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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel