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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140811134847.GF3249@zion.uk.xensource.com>
Date:	Mon, 11 Aug 2014 14:48:47 +0100
From:	Wei Liu <wei.liu2@...rix.com>
To:	David Vrabel <david.vrabel@...rix.com>
CC:	Wei Liu <wei.liu2@...rix.com>, <netdev@...r.kernel.org>,
	<xen-devel@...ts.xen.org>, Ian Campbell <ian.campbell@...rix.com>,
	Zoltan Kiss <zoltan.kiss@...rix.com>
Subject: Re: [Xen-devel] [PATCH net v2 2/3] xen-netback: don't stop dealloc
 kthread too early

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html