Re: [PATCH v3] u_ether: move hardware transmit to RX workqueue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Thu, Mar 06, 2014 at 11:42:49PM +0800, Weinn Jheng wrote:
> In order to reduce the interrupt times in the embedded system,
> a receiving workqueue is introduced.
> This modification also enhanced the overall throughput as the
> benefits of reducing interrupt occurrence.
> 
> Signed-off-by: Weinn Jheng <clanlab.proj@xxxxxxxxx>
> Cc: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
> Cc: David S. Miller <davem@xxxxxxxxxxxxx>

Dave, does this look ok from NAPI point of view ?

> Cc: Stephen Hemminger <shemminger@xxxxxxxxxx>
> Cc: Felipe Balbi <balbi@xxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> Changes for v3:
>    - Fix the From: and Signed-off-by: by Greg's remind.
> 
> Changes for v2:
>    - Remove the whitespace trailer.
>    - Reorganize the setup/destroy gether_wq work queue procedure
>      into APIs gether_setup and gether_cleanup
> 
>  drivers/usb/gadget/u_ether.c | 111 +++++++++++++++++++++++++++++--------------
>  1 file changed, 76 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
> index b7d4f82..506f16d 100644
> --- a/drivers/usb/gadget/u_ether.c
> +++ b/drivers/usb/gadget/u_ether.c
> @@ -72,6 +72,7 @@ struct eth_dev {
>  						struct sk_buff_head *list);
>  
>  	struct work_struct	work;
> +	struct work_struct	rx_work;
>  
>  	unsigned long		todo;
>  #define	WORK_RX_MEMORY		0
> @@ -81,6 +82,8 @@ struct eth_dev {
>  	u8			dev_mac[ETH_ALEN];
>  };
>  
> +static struct workqueue_struct	*gether_wq;
> +
>  /*-------------------------------------------------------------------------*/
>  
>  #define RX_EXTRA	20	/* bytes guarding against rx overflows */
> @@ -253,18 +256,16 @@ enomem:
>  		DBG(dev, "rx submit --> %d\n", retval);
>  		if (skb)
>  			dev_kfree_skb_any(skb);
> -		spin_lock_irqsave(&dev->req_lock, flags);
> -		list_add(&req->list, &dev->rx_reqs);
> -		spin_unlock_irqrestore(&dev->req_lock, flags);
>  	}
>  	return retval;
>  }
>  
>  static void rx_complete(struct usb_ep *ep, struct usb_request *req)
>  {
> -	struct sk_buff	*skb = req->context, *skb2;
> +	struct sk_buff	*skb = req->context;
>  	struct eth_dev	*dev = ep->driver_data;
>  	int		status = req->status;
> +	bool		rx_queue = 0;
>  
>  	switch (status) {
>  
> @@ -288,30 +289,8 @@ static void rx_complete(struct usb_ep *ep, struct usb_request *req)
>  		} else {
>  			skb_queue_tail(&dev->rx_frames, skb);
>  		}
> -		skb = NULL;
> -
> -		skb2 = skb_dequeue(&dev->rx_frames);
> -		while (skb2) {
> -			if (status < 0
> -					|| ETH_HLEN > skb2->len
> -					|| skb2->len > VLAN_ETH_FRAME_LEN) {
> -				dev->net->stats.rx_errors++;
> -				dev->net->stats.rx_length_errors++;
> -				DBG(dev, "rx length %d\n", skb2->len);
> -				dev_kfree_skb_any(skb2);
> -				goto next_frame;
> -			}
> -			skb2->protocol = eth_type_trans(skb2, dev->net);
> -			dev->net->stats.rx_packets++;
> -			dev->net->stats.rx_bytes += skb2->len;
> -
> -			/* no buffer copies needed, unless hardware can't
> -			 * use skb buffers.
> -			 */
> -			status = netif_rx(skb2);
> -next_frame:
> -			skb2 = skb_dequeue(&dev->rx_frames);
> -		}
> +		if (!status)
> +			rx_queue = 1;
>  		break;
>  
>  	/* software-driven interface shutdown */
> @@ -334,22 +313,20 @@ quiesce:
>  		/* FALLTHROUGH */
>  
>  	default:
> +		rx_queue = 1;
> +		dev_kfree_skb_any(skb);
>  		dev->net->stats.rx_errors++;
>  		DBG(dev, "rx status %d\n", status);
>  		break;
>  	}
>  
> -	if (skb)
> -		dev_kfree_skb_any(skb);
> -	if (!netif_running(dev->net)) {
>  clean:
>  		spin_lock(&dev->req_lock);
>  		list_add(&req->list, &dev->rx_reqs);
>  		spin_unlock(&dev->req_lock);
> -		req = NULL;
> -	}
> -	if (req)
> -		rx_submit(dev, req, GFP_ATOMIC);
> +
> +	if (rx_queue)
> +		queue_work(gether_wq, &dev->rx_work);
>  }
>  
>  static int prealloc(struct list_head *list, struct usb_ep *ep, unsigned n)
> @@ -414,16 +391,24 @@ static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags)
>  {
>  	struct usb_request	*req;
>  	unsigned long		flags;
> +	int			rx_counts = 0;
>  
>  	/* fill unused rxq slots with some skb */
>  	spin_lock_irqsave(&dev->req_lock, flags);
>  	while (!list_empty(&dev->rx_reqs)) {
> +
> +		if (++rx_counts > qlen(dev->gadget, dev->qmult))
> +			break;
> +
>  		req = container_of(dev->rx_reqs.next,
>  				struct usb_request, list);
>  		list_del_init(&req->list);
>  		spin_unlock_irqrestore(&dev->req_lock, flags);
>  
>  		if (rx_submit(dev, req, gfp_flags) < 0) {
> +			spin_lock_irqsave(&dev->req_lock, flags);
> +			list_add(&req->list, &dev->rx_reqs);
> +			spin_unlock_irqrestore(&dev->req_lock, flags);
>  			defer_kevent(dev, WORK_RX_MEMORY);
>  			return;
>  		}
> @@ -433,6 +418,36 @@ static void rx_fill(struct eth_dev *dev, gfp_t gfp_flags)
>  	spin_unlock_irqrestore(&dev->req_lock, flags);
>  }
>  
> +static void gether_rxwork(struct work_struct *work)
> +{
> +	struct eth_dev	*dev = container_of(work, struct eth_dev, rx_work);
> +	struct sk_buff	*skb;
> +	int		status = 0;
> +
> +	if (!dev->port_usb)
> +		return;
> +
> +	while ((skb = skb_dequeue(&dev->rx_frames))) {
> +		if (status < 0
> +				|| ETH_HLEN > skb->len
> +				|| skb->len > VLAN_ETH_FRAME_LEN) {
> +			dev->net->stats.rx_errors++;
> +			dev->net->stats.rx_length_errors++;
> +			DBG(dev, "rx length %d\n", skb->len);
> +			dev_kfree_skb_any(skb);
> +			continue;
> +		}
> +		skb->protocol = eth_type_trans(skb, dev->net);
> +		dev->net->stats.rx_packets++;
> +		dev->net->stats.rx_bytes += skb->len;
> +
> +		status = netif_rx_ni(skb);
> +	}
> +
> +	if (netif_running(dev->net))
> +		rx_fill(dev, GFP_KERNEL);
> +}
> +
>  static void eth_work(struct work_struct *work)
>  {
>  	struct eth_dev	*dev = container_of(work, struct eth_dev, work);
> @@ -767,10 +782,17 @@ struct eth_dev *gether_setup_name(struct usb_gadget *g,
>  	if (!net)
>  		return ERR_PTR(-ENOMEM);
>  
> +	gether_wq  = create_singlethread_workqueue("gether");
> +	if (!gether_wq) {
> +		pr_err("Cannot initialize work queue\n");
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
>  	dev = netdev_priv(net);
>  	spin_lock_init(&dev->lock);
>  	spin_lock_init(&dev->req_lock);
>  	INIT_WORK(&dev->work, eth_work);
> +	INIT_WORK(&dev->rx_work, gether_rxwork);
>  	INIT_LIST_HEAD(&dev->tx_reqs);
>  	INIT_LIST_HEAD(&dev->rx_reqs);
>  
> @@ -829,10 +851,17 @@ struct net_device *gether_setup_name_default(const char *netname)
>  	if (!net)
>  		return ERR_PTR(-ENOMEM);
>  
> +	gether_wq  = create_singlethread_workqueue("gether");
> +	if (!gether_wq) {
> +		pr_err("Cannot initialize work queue\n");
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
>  	dev = netdev_priv(net);
>  	spin_lock_init(&dev->lock);
>  	spin_lock_init(&dev->req_lock);
>  	INIT_WORK(&dev->work, eth_work);
> +	INIT_WORK(&dev->rx_work, gether_rxwork);
>  	INIT_LIST_HEAD(&dev->tx_reqs);
>  	INIT_LIST_HEAD(&dev->rx_reqs);
>  
> @@ -1010,6 +1039,11 @@ void gether_cleanup(struct eth_dev *dev)
>  	if (!dev)
>  		return;
>  
> +	if (gether_wq) {
> +		flush_work(&dev->rx_work);
> +		destroy_workqueue(gether_wq);
> +	}
> +
>  	unregister_netdev(dev->net);
>  	flush_work(&dev->work);
>  	free_netdev(dev->net);
> @@ -1113,6 +1147,7 @@ void gether_disconnect(struct gether *link)
>  {
>  	struct eth_dev		*dev = link->ioport;
>  	struct usb_request	*req;
> +	struct sk_buff		*skb;
>  
>  	WARN_ON(!dev);
>  	if (!dev)
> @@ -1139,6 +1174,12 @@ void gether_disconnect(struct gether *link)
>  		spin_lock(&dev->req_lock);
>  	}
>  	spin_unlock(&dev->req_lock);
> +
> +	spin_lock(&dev->rx_frames.lock);
> +	while ((skb = __skb_dequeue(&dev->rx_frames)))
> +		dev_kfree_skb_any(skb);
> +	spin_unlock(&dev->rx_frames.lock);
> +
>  	link->in_ep->driver_data = NULL;
>  	link->in_ep->desc = NULL;
>  
> -- 
> 1.9.rc1
> 

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux