Re: [Xen-devel] [PATCH net v2 2/3] xen-netback: don't stop dealloc kthread too early

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Aug 11, 2014 at 01:10:12PM +0100, David Vrabel wrote:
> On 08/08/14 17:37, Wei Liu wrote:
[...]
> >     if (tx_evtchn == rx_evtchn) {
> >             /* feature-split-event-channels == 0 */
> > @@ -687,6 +700,9 @@ void xenvif_disconnect(struct xenvif *vif)
> >                     queue->task = NULL;
> >             }
> >  
> > +           wait_event(queue->inflight_wq,
> > +                      atomic_read(&queue->inflight_packets) == 0);
> 
> Just make the dealloc task not stop unless it has deallocated all
> outstanding requests.  There's no need for another wait queue here.
> 

Are you suggesting something like

  while (atomic_read(&queue->inflight_packets) !=0)
        schedule_timeout(SOME_TIMEOUT);

?

> > +
> >             if (queue->dealloc_task) {
> >                     kthread_stop(queue->dealloc_task);
> >                     queue->dealloc_task = NULL;
> > diff --git a/drivers/net/xen-netback/netback.c 
> > b/drivers/net/xen-netback/netback.c
> > index 4734472..d2f0c7d7 100644
> > --- a/drivers/net/xen-netback/netback.c
> > +++ b/drivers/net/xen-netback/netback.c
> > @@ -107,6 +107,18 @@ static inline unsigned long idx_to_kaddr(struct 
> > xenvif_queue *queue,
> >  #define callback_param(vif, pending_idx) \
> >     (vif->pending_tx_info[pending_idx].callback_struct)
> >  
> > +/* This function is used to set SKBTX_DEV_ZEROCOPY as well as
> > + * increasing the inflight counter. We need to increase the inflight
> > + * counter because core driver calls into xenvif_zerocopy_callback
> > + * which calls xenvif_dec_inflight_packets.
> > + */
> > +static void set_skb_zerocopy(struct xenvif_queue *queue,
> > +                        struct sk_buff *skb)
> > +{
> > +   skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
> > +   xenvif_inc_inflight_packets(queue);
> > +}
> 
> This name makes this look like a core function instead of a netback
> specific one.
> 
> I would suggest a pair of functions:
> 
> xenvif_skb_zerocopy_prepare()
> xenvif_skb_zerocopy_complete()
> 

This will do.

Wei.

> Or similar.
> 
> David

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