[PATCH net-next 0/2] Xen network: split event channels support

[PATCH net-next 0/2] Xen network: split event channels support

[Wei Liu]

This series adds a new feature called split event channels.

In the original implementation, only one event channel is setup between
frontend and backend. This is not ideal as TX notification interferes with RX
notification. Using dedicated event channels for TX and RX solves this issue.

Wei Liu (2):
  xen-netback: split event channels feature support
  xen-netfront: split event channels feature support

 drivers/net/xen-netback/common.h    |   11 ++-
 drivers/net/xen-netback/interface.c |   87 ++++++++++++++---
 drivers/net/xen-netback/netback.c   |    7 +-
 drivers/net/xen-netback/xenbus.c    |   40 ++++++--
 drivers/net/xen-netfront.c          |  181 +++++++++++++++++++++++++++++------
 5 files changed, 269 insertions(+), 57 deletions(-)

-- 
1.7.10.4

[PATCH net-next 1/2] xen-netback: split event channels feature support

[Wei Liu]

Netback and netfront only use one event channel to do TX / RX notification,
which may cause unnecessary wake-up of processing routines. This patch adds a
new feature called feature-split-event-channels to netback, enabling it to
handle TX and RX events separately.

Netback will use tx_irq to notify guest for TX completion, rx_irq for RX
notification.

If frontend doesn't support this feature, tx_irq is equal to rx_irq.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 drivers/net/xen-netback/common.h    |   11 +++--
 drivers/net/xen-netback/interface.c |   87 +++++++++++++++++++++++++++++------
 drivers/net/xen-netback/netback.c   |    7 ++-
 drivers/net/xen-netback/xenbus.c    |   40 +++++++++++++---
 4 files changed, 117 insertions(+), 28 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 6102a6c..ac82042 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -57,8 +57,12 @@ struct xenvif {
 
 	u8               fe_dev_addr[6];
 
-	/* Physical parameters of the comms window. */
-	unsigned int     irq;
+	/* When feature-split-event-channels = 0, tx_irq = rx_irq. */
+	unsigned int tx_irq;
+	unsigned int rx_irq;
+	/* Only used when feature-split-event-channels = 1 */
+	char tx_irq_name[IFNAMSIZ+4]; /* DEVNAME-tx */
+	char rx_irq_name[IFNAMSIZ+4]; /* DEVNAME-rx */
 
 	/* List of frontends to notify after a batch of frames sent. */
 	struct list_head notify_list;
@@ -113,7 +117,8 @@ struct xenvif *xenvif_alloc(struct device *parent,
 			    unsigned int handle);
 
 int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
-		   unsigned long rx_ring_ref, unsigned int evtchn);
+		   unsigned long rx_ring_ref, unsigned int tx_evtchn,
+		   unsigned int rx_evtchn);
 void xenvif_disconnect(struct xenvif *vif);
 
 void xenvif_get(struct xenvif *vif);
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 82202c2..bbed5da 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -60,7 +60,7 @@ static int xenvif_rx_schedulable(struct xenvif *vif)
 	return xenvif_schedulable(vif) && !xen_netbk_rx_ring_full(vif);
 }
 
-static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
+static irqreturn_t xenvif_tx_interrupt(int irq, void *dev_id)
 {
 	struct xenvif *vif = dev_id;
 
@@ -69,12 +69,35 @@ static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
 
 	xen_netbk_schedule_xenvif(vif);
 
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
+{
+	struct xenvif *vif = dev_id;
+
+	if (vif->netbk == NULL)
+		return IRQ_NONE;
+
 	if (xenvif_rx_schedulable(vif))
 		netif_wake_queue(vif->dev);
 
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
+{
+	struct xenvif *vif = dev_id;
+
+	if (vif->netbk == NULL)
+		return IRQ_NONE;
+
+	xenvif_tx_interrupt(irq, dev_id);
+	xenvif_rx_interrupt(irq, dev_id);
+
+	return IRQ_HANDLED;
+}
+
 static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct xenvif *vif = netdev_priv(dev);
@@ -125,13 +148,15 @@ static struct net_device_stats *xenvif_get_stats(struct net_device *dev)
 static void xenvif_up(struct xenvif *vif)
 {
 	xen_netbk_add_xenvif(vif);
-	enable_irq(vif->irq);
+	enable_irq(vif->tx_irq);
+	enable_irq(vif->rx_irq);
 	xen_netbk_check_rx_xenvif(vif);
 }
 
 static void xenvif_down(struct xenvif *vif)
 {
-	disable_irq(vif->irq);
+	disable_irq(vif->tx_irq);
+	disable_irq(vif->rx_irq);
 	del_timer_sync(&vif->credit_timeout);
 	xen_netbk_deschedule_xenvif(vif);
 	xen_netbk_remove_xenvif(vif);
@@ -308,12 +333,13 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 }
 
 int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
-		   unsigned long rx_ring_ref, unsigned int evtchn)
+		   unsigned long rx_ring_ref, unsigned int tx_evtchn,
+		   unsigned int rx_evtchn)
 {
 	int err = -ENOMEM;
 
 	/* Already connected through? */
-	if (vif->irq)
+	if (vif->tx_irq)
 		return 0;
 
 	__module_get(THIS_MODULE);
@@ -322,13 +348,38 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 	if (err < 0)
 		goto err;
 
-	err = bind_interdomain_evtchn_to_irqhandler(
-		vif->domid, evtchn, xenvif_interrupt, 0,
-		vif->dev->name, vif);
-	if (err < 0)
-		goto err_unmap;
-	vif->irq = err;
-	disable_irq(vif->irq);
+	if (tx_evtchn == rx_evtchn) {
+		/* feature-split-event-channels == 0 */
+		err = bind_interdomain_evtchn_to_irqhandler(
+			vif->domid, tx_evtchn, xenvif_interrupt, 0,
+			vif->dev->name, vif);
+		if (err < 0)
+			goto err_unmap;
+		vif->tx_irq = vif->rx_irq = err;
+		disable_irq(vif->tx_irq);
+		disable_irq(vif->rx_irq);
+	} else {
+		/* feature-split-event-channels == 1 */
+		snprintf(vif->tx_irq_name, sizeof(vif->tx_irq_name),
+			 "%s-tx", vif->dev->name);
+		err = bind_interdomain_evtchn_to_irqhandler(
+			vif->domid, tx_evtchn, xenvif_tx_interrupt, 0,
+			vif->tx_irq_name, vif);
+		if (err < 0)
+			goto err_unmap;
+		vif->tx_irq = err;
+		disable_irq(vif->tx_irq);
+
+		snprintf(vif->rx_irq_name, sizeof(vif->rx_irq_name),
+			 "%s-rx", vif->dev->name);
+		err = bind_interdomain_evtchn_to_irqhandler(
+			vif->domid, rx_evtchn, xenvif_rx_interrupt, 0,
+			vif->rx_irq_name, vif);
+		if (err < 0)
+			goto err_tx_unbind;
+		vif->rx_irq = err;
+		disable_irq(vif->rx_irq);
+	}
 
 	xenvif_get(vif);
 
@@ -342,6 +393,9 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 	rtnl_unlock();
 
 	return 0;
+err_tx_unbind:
+	unbind_from_irqhandler(vif->tx_irq, vif);
+	vif->tx_irq = 0;
 err_unmap:
 	xen_netbk_unmap_frontend_rings(vif);
 err:
@@ -375,8 +429,13 @@ void xenvif_disconnect(struct xenvif *vif)
 	atomic_dec(&vif->refcnt);
 	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
 
-	if (vif->irq) {
-		unbind_from_irqhandler(vif->irq, vif);
+	if (vif->tx_irq) {
+		if (vif->tx_irq == vif->rx_irq)
+			unbind_from_irqhandler(vif->tx_irq, vif);
+		else {
+			unbind_from_irqhandler(vif->tx_irq, vif);
+			unbind_from_irqhandler(vif->rx_irq, vif);
+		}
 		/* vif->irq is valid, we had a module_get in
 		 * xenvif_connect.
 		 */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 2d9477f..5f858bc 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -662,7 +662,7 @@ static void xen_netbk_rx_action(struct xen_netbk *netbk)
 {
 	struct xenvif *vif = NULL, *tmp;
 	s8 status;
-	u16 irq, flags;
+	u16 flags;
 	struct xen_netif_rx_response *resp;
 	struct sk_buff_head rxq;
 	struct sk_buff *skb;
@@ -771,7 +771,6 @@ static void xen_netbk_rx_action(struct xen_netbk *netbk)
 					 sco->meta_slots_used);
 
 		RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&vif->rx, ret);
-		irq = vif->irq;
 		if (ret && list_empty(&vif->notify_list))
 			list_add_tail(&vif->notify_list, &notify);
 
@@ -783,7 +782,7 @@ static void xen_netbk_rx_action(struct xen_netbk *netbk)
 	}
 
 	list_for_each_entry_safe(vif, tmp, &notify, notify_list) {
-		notify_remote_via_irq(vif->irq);
+		notify_remote_via_irq(vif->rx_irq);
 		list_del_init(&vif->notify_list);
 	}
 
@@ -1762,7 +1761,7 @@ static void make_tx_response(struct xenvif *vif,
 	vif->tx.rsp_prod_pvt = ++i;
 	RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&vif->tx, notify);
 	if (notify)
-		notify_remote_via_irq(vif->irq);
+		notify_remote_via_irq(vif->tx_irq);
 }
 
 static struct xen_netif_rx_response *make_rx_response(struct xenvif *vif,
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index d230c14..79b40db 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -114,6 +114,15 @@ static int netback_probe(struct xenbus_device *dev,
 			goto abort_transaction;
 		}
 
+		/* Split event channels support */
+		err = xenbus_printf(xbt, dev->nodename,
+				"feature-split-event-channels",
+				"%u", 1);
+		if (err) {
+			message = "writing feature-split-event-channels";
+			goto abort_transaction;
+		}
+
 		err = xenbus_transaction_end(xbt, 0);
 	} while (err == -EAGAIN);
 
@@ -393,21 +402,36 @@ static int connect_rings(struct backend_info *be)
 	struct xenvif *vif = be->vif;
 	struct xenbus_device *dev = be->dev;
 	unsigned long tx_ring_ref, rx_ring_ref;
-	unsigned int evtchn, rx_copy;
+	unsigned int tx_evtchn, rx_evtchn, rx_copy;
 	int err;
 	int val;
 
 	err = xenbus_gather(XBT_NIL, dev->otherend,
 			    "tx-ring-ref", "%lu", &tx_ring_ref,
-			    "rx-ring-ref", "%lu", &rx_ring_ref,
-			    "event-channel", "%u", &evtchn, NULL);
+			    "rx-ring-ref", "%lu", &rx_ring_ref, NULL);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
-				 "reading %s/ring-ref and event-channel",
+				 "reading %s/ring-ref",
 				 dev->otherend);
 		return err;
 	}
 
+	/* Try split event channels first, then single event channel. */
+	err = xenbus_gather(XBT_NIL, dev->otherend,
+			    "event-channel-tx", "%u", &tx_evtchn,
+			    "event-channel-rx", "%u", &rx_evtchn, NULL);
+	if (err) {
+		err = xenbus_scanf(XBT_NIL, dev->otherend,
+				   "event-channel", "%u", &tx_evtchn);
+		if (err) {
+			xenbus_dev_fatal(dev, err,
+					 "reading %s/event-channel(-tx/rx)",
+					 dev->otherend);
+			return err;
+		}
+		rx_evtchn = tx_evtchn;
+	}
+
 	err = xenbus_scanf(XBT_NIL, dev->otherend, "request-rx-copy", "%u",
 			   &rx_copy);
 	if (err == -ENOENT) {
@@ -454,11 +478,13 @@ static int connect_rings(struct backend_info *be)
 	vif->csum = !val;
 
 	/* Map the shared frame, irq etc. */
-	err = xenvif_connect(vif, tx_ring_ref, rx_ring_ref, evtchn);
+	err = xenvif_connect(vif, tx_ring_ref, rx_ring_ref,
+			     tx_evtchn, rx_evtchn);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
-				 "mapping shared-frames %lu/%lu port %u",
-				 tx_ring_ref, rx_ring_ref, evtchn);
+				 "mapping shared-frames %lu/%lu port tx %u rx %u",
+				 tx_ring_ref, rx_ring_ref,
+				 tx_evtchn, rx_evtchn);
 		return err;
 	}
 	return 0;
-- 
1.7.10.4

[PATCH net-next 1/2] xen-netback: split event channels feature support

[Wei Liu]

Netback and netfront only use one event channel to do TX / RX notification,
which may cause unnecessary wake-up of processing routines. This patch adds a
new feature called feature-split-event-channels to netback, enabling it to
handle TX and RX events separately.

Netback will use tx_irq to notify guest for TX completion, rx_irq for RX
notification.

If frontend doesn't support this feature, tx_irq is equal to rx_irq.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 drivers/net/xen-netback/common.h    |   11 +++--
 drivers/net/xen-netback/interface.c |   87 +++++++++++++++++++++++++++++------
 drivers/net/xen-netback/netback.c   |    7 ++-
 drivers/net/xen-netback/xenbus.c    |   40 +++++++++++++---
 4 files changed, 117 insertions(+), 28 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 6102a6c..ac82042 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -57,8 +57,12 @@ struct xenvif {
 
 	u8               fe_dev_addr[6];
 
-	/* Physical parameters of the comms window. */
-	unsigned int     irq;
+	/* When feature-split-event-channels = 0, tx_irq = rx_irq. */
+	unsigned int tx_irq;
+	unsigned int rx_irq;
+	/* Only used when feature-split-event-channels = 1 */
+	char tx_irq_name[IFNAMSIZ+4]; /* DEVNAME-tx */
+	char rx_irq_name[IFNAMSIZ+4]; /* DEVNAME-rx */
 
 	/* List of frontends to notify after a batch of frames sent. */
 	struct list_head notify_list;
@@ -113,7 +117,8 @@ struct xenvif *xenvif_alloc(struct device *parent,
 			    unsigned int handle);
 
 int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
-		   unsigned long rx_ring_ref, unsigned int evtchn);
+		   unsigned long rx_ring_ref, unsigned int tx_evtchn,
+		   unsigned int rx_evtchn);
 void xenvif_disconnect(struct xenvif *vif);
 
 void xenvif_get(struct xenvif *vif);
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 82202c2..bbed5da 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -60,7 +60,7 @@ static int xenvif_rx_schedulable(struct xenvif *vif)
 	return xenvif_schedulable(vif) && !xen_netbk_rx_ring_full(vif);
 }
 
-static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
+static irqreturn_t xenvif_tx_interrupt(int irq, void *dev_id)
 {
 	struct xenvif *vif = dev_id;
 
@@ -69,12 +69,35 @@ static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
 
 	xen_netbk_schedule_xenvif(vif);
 
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
+{
+	struct xenvif *vif = dev_id;
+
+	if (vif->netbk == NULL)
+		return IRQ_NONE;
+
 	if (xenvif_rx_schedulable(vif))
 		netif_wake_queue(vif->dev);
 
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
+{
+	struct xenvif *vif = dev_id;
+
+	if (vif->netbk == NULL)
+		return IRQ_NONE;
+
+	xenvif_tx_interrupt(irq, dev_id);
+	xenvif_rx_interrupt(irq, dev_id);
+
+	return IRQ_HANDLED;
+}
+
 static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct xenvif *vif = netdev_priv(dev);
@@ -125,13 +148,15 @@ static struct net_device_stats *xenvif_get_stats(struct net_device *dev)
 static void xenvif_up(struct xenvif *vif)
 {
 	xen_netbk_add_xenvif(vif);
-	enable_irq(vif->irq);
+	enable_irq(vif->tx_irq);
+	enable_irq(vif->rx_irq);
 	xen_netbk_check_rx_xenvif(vif);
 }
 
 static void xenvif_down(struct xenvif *vif)
 {
-	disable_irq(vif->irq);
+	disable_irq(vif->tx_irq);
+	disable_irq(vif->rx_irq);
 	del_timer_sync(&vif->credit_timeout);
 	xen_netbk_deschedule_xenvif(vif);
 	xen_netbk_remove_xenvif(vif);
@@ -308,12 +333,13 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 }
 
 int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
-		   unsigned long rx_ring_ref, unsigned int evtchn)
+		   unsigned long rx_ring_ref, unsigned int tx_evtchn,
+		   unsigned int rx_evtchn)
 {
 	int err = -ENOMEM;
 
 	/* Already connected through? */
-	if (vif->irq)
+	if (vif->tx_irq)
 		return 0;
 
 	__module_get(THIS_MODULE);
@@ -322,13 +348,38 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 	if (err < 0)
 		goto err;
 
-	err = bind_interdomain_evtchn_to_irqhandler(
-		vif->domid, evtchn, xenvif_interrupt, 0,
-		vif->dev->name, vif);
-	if (err < 0)
-		goto err_unmap;
-	vif->irq = err;
-	disable_irq(vif->irq);
+	if (tx_evtchn == rx_evtchn) {
+		/* feature-split-event-channels == 0 */
+		err = bind_interdomain_evtchn_to_irqhandler(
+			vif->domid, tx_evtchn, xenvif_interrupt, 0,
+			vif->dev->name, vif);
+		if (err < 0)
+			goto err_unmap;
+		vif->tx_irq = vif->rx_irq = err;
+		disable_irq(vif->tx_irq);
+		disable_irq(vif->rx_irq);
+	} else {
+		/* feature-split-event-channels == 1 */
+		snprintf(vif->tx_irq_name, sizeof(vif->tx_irq_name),
+			 "%s-tx", vif->dev->name);
+		err = bind_interdomain_evtchn_to_irqhandler(
+			vif->domid, tx_evtchn, xenvif_tx_interrupt, 0,
+			vif->tx_irq_name, vif);
+		if (err < 0)
+			goto err_unmap;
+		vif->tx_irq = err;
+		disable_irq(vif->tx_irq);
+
+		snprintf(vif->rx_irq_name, sizeof(vif->rx_irq_name),
+			 "%s-rx", vif->dev->name);
+		err = bind_interdomain_evtchn_to_irqhandler(
+			vif->domid, rx_evtchn, xenvif_rx_interrupt, 0,
+			vif->rx_irq_name, vif);
+		if (err < 0)
+			goto err_tx_unbind;
+		vif->rx_irq = err;
+		disable_irq(vif->rx_irq);
+	}
 
 	xenvif_get(vif);
 
@@ -342,6 +393,9 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 	rtnl_unlock();
 
 	return 0;
+err_tx_unbind:
+	unbind_from_irqhandler(vif->tx_irq, vif);
+	vif->tx_irq = 0;
 err_unmap:
 	xen_netbk_unmap_frontend_rings(vif);
 err:
@@ -375,8 +429,13 @@ void xenvif_disconnect(struct xenvif *vif)
 	atomic_dec(&vif->refcnt);
 	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
 
-	if (vif->irq) {
-		unbind_from_irqhandler(vif->irq, vif);
+	if (vif->tx_irq) {
+		if (vif->tx_irq == vif->rx_irq)
+			unbind_from_irqhandler(vif->tx_irq, vif);
+		else {
+			unbind_from_irqhandler(vif->tx_irq, vif);
+			unbind_from_irqhandler(vif->rx_irq, vif);
+		}
 		/* vif->irq is valid, we had a module_get in
 		 * xenvif_connect.
 		 */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 2d9477f..5f858bc 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -662,7 +662,7 @@ static void xen_netbk_rx_action(struct xen_netbk *netbk)
 {
 	struct xenvif *vif = NULL, *tmp;
 	s8 status;
-	u16 irq, flags;
+	u16 flags;
 	struct xen_netif_rx_response *resp;
 	struct sk_buff_head rxq;
 	struct sk_buff *skb;
@@ -771,7 +771,6 @@ static void xen_netbk_rx_action(struct xen_netbk *netbk)
 					 sco->meta_slots_used);
 
 		RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&vif->rx, ret);
-		irq = vif->irq;
 		if (ret && list_empty(&vif->notify_list))
 			list_add_tail(&vif->notify_list, &notify);
 
@@ -783,7 +782,7 @@ static void xen_netbk_rx_action(struct xen_netbk *netbk)
 	}
 
 	list_for_each_entry_safe(vif, tmp, &notify, notify_list) {
-		notify_remote_via_irq(vif->irq);
+		notify_remote_via_irq(vif->rx_irq);
 		list_del_init(&vif->notify_list);
 	}
 
@@ -1762,7 +1761,7 @@ static void make_tx_response(struct xenvif *vif,
 	vif->tx.rsp_prod_pvt = ++i;
 	RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&vif->tx, notify);
 	if (notify)
-		notify_remote_via_irq(vif->irq);
+		notify_remote_via_irq(vif->tx_irq);
 }
 
 static struct xen_netif_rx_response *make_rx_response(struct xenvif *vif,
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index d230c14..79b40db 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -114,6 +114,15 @@ static int netback_probe(struct xenbus_device *dev,
 			goto abort_transaction;
 		}
 
+		/* Split event channels support */
+		err = xenbus_printf(xbt, dev->nodename,
+				"feature-split-event-channels",
+				"%u", 1);
+		if (err) {
+			message = "writing feature-split-event-channels";
+			goto abort_transaction;
+		}
+
 		err = xenbus_transaction_end(xbt, 0);
 	} while (err == -EAGAIN);
 
@@ -393,21 +402,36 @@ static int connect_rings(struct backend_info *be)
 	struct xenvif *vif = be->vif;
 	struct xenbus_device *dev = be->dev;
 	unsigned long tx_ring_ref, rx_ring_ref;
-	unsigned int evtchn, rx_copy;
+	unsigned int tx_evtchn, rx_evtchn, rx_copy;
 	int err;
 	int val;
 
 	err = xenbus_gather(XBT_NIL, dev->otherend,
 			    "tx-ring-ref", "%lu", &tx_ring_ref,
-			    "rx-ring-ref", "%lu", &rx_ring_ref,
-			    "event-channel", "%u", &evtchn, NULL);
+			    "rx-ring-ref", "%lu", &rx_ring_ref, NULL);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
-				 "reading %s/ring-ref and event-channel",
+				 "reading %s/ring-ref",
 				 dev->otherend);
 		return err;
 	}
 
+	/* Try split event channels first, then single event channel. */
+	err = xenbus_gather(XBT_NIL, dev->otherend,
+			    "event-channel-tx", "%u", &tx_evtchn,
+			    "event-channel-rx", "%u", &rx_evtchn, NULL);
+	if (err) {
+		err = xenbus_scanf(XBT_NIL, dev->otherend,
+				   "event-channel", "%u", &tx_evtchn);
+		if (err) {
+			xenbus_dev_fatal(dev, err,
+					 "reading %s/event-channel(-tx/rx)",
+					 dev->otherend);
+			return err;
+		}
+		rx_evtchn = tx_evtchn;
+	}
+
 	err = xenbus_scanf(XBT_NIL, dev->otherend, "request-rx-copy", "%u",
 			   &rx_copy);
 	if (err == -ENOENT) {
@@ -454,11 +478,13 @@ static int connect_rings(struct backend_info *be)
 	vif->csum = !val;
 
 	/* Map the shared frame, irq etc. */
-	err = xenvif_connect(vif, tx_ring_ref, rx_ring_ref, evtchn);
+	err = xenvif_connect(vif, tx_ring_ref, rx_ring_ref,
+			     tx_evtchn, rx_evtchn);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
-				 "mapping shared-frames %lu/%lu port %u",
-				 tx_ring_ref, rx_ring_ref, evtchn);
+				 "mapping shared-frames %lu/%lu port tx %u rx %u",
+				 tx_ring_ref, rx_ring_ref,
+				 tx_evtchn, rx_evtchn);
 		return err;
 	}
 	return 0;
-- 
1.7.10.4

Re: [PATCH net-next 1/2] xen-netback: split event channels feature support

[David Vrabel]

On 21/05/13 00:02, Wei Liu wrote:
> Netback and netfront only use one event channel to do TX / RX notification,
> which may cause unnecessary wake-up of processing routines. This patch adds a
> new feature called feature-split-event-channels to netback, enabling it to
> handle TX and RX events separately.
> 
> Netback will use tx_irq to notify guest for TX completion, rx_irq for RX
> notification.
> 
> If frontend doesn't support this feature, tx_irq is equal to rx_irq.
[...]
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -69,12 +69,35 @@ static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
>  
>  	xen_netbk_schedule_xenvif(vif);
>  
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
> +{
> +	struct xenvif *vif = dev_id;
> +
> +	if (vif->netbk == NULL)
> +		return IRQ_NONE;

I know the original code had this but this looks suspect to me.  Event
channels are never shared so it does not make sense to ever return IRQ_NONE.

Suggest making this return IRQ_HANDLED instead (and similarly in
xenvif_tx_interrupt()) and...

> +static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
> +{
> +	struct xenvif *vif = dev_id;
> +
> +	if (vif->netbk == NULL)
> +		return IRQ_NONE;

... then you can remove this test.

> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -114,6 +114,15 @@ static int netback_probe(struct xenbus_device *dev,
>  			goto abort_transaction;
>  		}
>  
> +		/* Split event channels support */
> +		err = xenbus_printf(xbt, dev->nodename,
> +				"feature-split-event-channels",
> +				"%u", 1);
> +		if (err) {
> +			message = "writing feature-split-event-channels";
> +			goto abort_transaction;
> +		}
> +

Event channels are currently a limited resource.  Do we want to have a
knob to disable this feature?

David

Re: [Xen-devel] [PATCH net-next 1/2] xen-netback: split event channels feature support

[David Vrabel]

On 21/05/13 00:02, Wei Liu wrote:
> Netback and netfront only use one event channel to do TX / RX notification,
> which may cause unnecessary wake-up of processing routines. This patch adds a
> new feature called feature-split-event-channels to netback, enabling it to
> handle TX and RX events separately.
> 
> Netback will use tx_irq to notify guest for TX completion, rx_irq for RX
> notification.
> 
> If frontend doesn't support this feature, tx_irq is equal to rx_irq.
[...]
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -69,12 +69,35 @@ static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
>  
>  	xen_netbk_schedule_xenvif(vif);
>  
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
> +{
> +	struct xenvif *vif = dev_id;
> +
> +	if (vif->netbk == NULL)
> +		return IRQ_NONE;

I know the original code had this but this looks suspect to me.  Event
channels are never shared so it does not make sense to ever return IRQ_NONE.

Suggest making this return IRQ_HANDLED instead (and similarly in
xenvif_tx_interrupt()) and...

> +static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
> +{
> +	struct xenvif *vif = dev_id;
> +
> +	if (vif->netbk == NULL)
> +		return IRQ_NONE;

... then you can remove this test.

> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -114,6 +114,15 @@ static int netback_probe(struct xenbus_device *dev,
>  			goto abort_transaction;
>  		}
>  
> +		/* Split event channels support */
> +		err = xenbus_printf(xbt, dev->nodename,
> +				"feature-split-event-channels",
> +				"%u", 1);
> +		if (err) {
> +			message = "writing feature-split-event-channels";
> +			goto abort_transaction;
> +		}
> +

Event channels are currently a limited resource.  Do we want to have a
knob to disable this feature?

David

Re: [Xen-devel] [PATCH net-next 1/2] xen-netback: split event channels feature support

[Wei Liu]

On Tue, May 21, 2013 at 02:34:19PM +0100, David Vrabel wrote:
> > +
> > +static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
> > +{
> > +	struct xenvif *vif = dev_id;
> > +
> > +	if (vif->netbk == NULL)
> > +		return IRQ_NONE;
> 
> I know the original code had this but this looks suspect to me.  Event
> channels are never shared so it does not make sense to ever return IRQ_NONE.
> 
> Suggest making this return IRQ_HANDLED instead (and similarly in
> xenvif_tx_interrupt()) and...
> 

Fixed.

> > +static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
> > +{
> > +	struct xenvif *vif = dev_id;
> > +
> > +	if (vif->netbk == NULL)
> > +		return IRQ_NONE;
> 
> ... then you can remove this test.

Fixed.

> 
> > --- a/drivers/net/xen-netback/xenbus.c
> > +++ b/drivers/net/xen-netback/xenbus.c
> > @@ -114,6 +114,15 @@ static int netback_probe(struct xenbus_device *dev,
> >  			goto abort_transaction;
> >  		}
> >  
> > +		/* Split event channels support */
> > +		err = xenbus_printf(xbt, dev->nodename,
> > +				"feature-split-event-channels",
> > +				"%u", 1);
> > +		if (err) {
> > +			message = "writing feature-split-event-channels";
> > +			goto abort_transaction;
> > +		}
> > +
> 
> Event channels are currently a limited resource.  Do we want to have a
> knob to disable this feature?

Knob feature_split_event_channels added.


Wei.

Re: [PATCH net-next 1/2] xen-netback: split event channels feature support

[Wei Liu]

On Tue, May 21, 2013 at 02:34:19PM +0100, David Vrabel wrote:
> > +
> > +static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
> > +{
> > +	struct xenvif *vif = dev_id;
> > +
> > +	if (vif->netbk == NULL)
> > +		return IRQ_NONE;
> 
> I know the original code had this but this looks suspect to me.  Event
> channels are never shared so it does not make sense to ever return IRQ_NONE.
> 
> Suggest making this return IRQ_HANDLED instead (and similarly in
> xenvif_tx_interrupt()) and...
> 

Fixed.

> > +static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
> > +{
> > +	struct xenvif *vif = dev_id;
> > +
> > +	if (vif->netbk == NULL)
> > +		return IRQ_NONE;
> 
> ... then you can remove this test.

Fixed.

> 
> > --- a/drivers/net/xen-netback/xenbus.c
> > +++ b/drivers/net/xen-netback/xenbus.c
> > @@ -114,6 +114,15 @@ static int netback_probe(struct xenbus_device *dev,
> >  			goto abort_transaction;
> >  		}
> >  
> > +		/* Split event channels support */
> > +		err = xenbus_printf(xbt, dev->nodename,
> > +				"feature-split-event-channels",
> > +				"%u", 1);
> > +		if (err) {
> > +			message = "writing feature-split-event-channels";
> > +			goto abort_transaction;
> > +		}
> > +
> 
> Event channels are currently a limited resource.  Do we want to have a
> knob to disable this feature?

Knob feature_split_event_channels added.


Wei.

[PATCH net-next 2/2] xen-netfront: split event channels feature support

[Wei Liu]

This patch adds a new feature called feature-split-event-channels for
netfront, enabling it to handle TX and RX events separately.

If netback does not support this feature, it falls back to use single event
channel.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 drivers/net/xen-netfront.c |  181 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 152 insertions(+), 29 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 5770e3b..716b2bd 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -85,7 +85,15 @@ struct netfront_info {
 
 	struct napi_struct napi;
 
-	unsigned int evtchn;
+	/* Split event channels support, tx_* == rx_* when using
+	 * single event channel.
+	 */
+	unsigned int tx_evtchn, rx_evtchn;
+	unsigned int tx_irq, rx_irq;
+	/* Only used when split event channels support is enabled */
+	char tx_irq_name[IFNAMSIZ+4]; /* DEVNAME-tx */
+	char rx_irq_name[IFNAMSIZ+4]; /* DEVNAME-rx */
+
 	struct xenbus_device *xbdev;
 
 	spinlock_t   tx_lock;
@@ -330,7 +338,7 @@ no_skb:
  push:
 	RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&np->rx, notify);
 	if (notify)
-		notify_remote_via_irq(np->netdev->irq);
+		notify_remote_via_irq(np->rx_irq);
 }
 
 static int xennet_open(struct net_device *dev)
@@ -623,7 +631,7 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&np->tx, notify);
 	if (notify)
-		notify_remote_via_irq(np->netdev->irq);
+		notify_remote_via_irq(np->tx_irq);
 
 	u64_stats_update_begin(&stats->syncp);
 	stats->tx_bytes += skb->len;
@@ -1254,23 +1262,35 @@ static int xennet_set_features(struct net_device *dev,
 	return 0;
 }
 
-static irqreturn_t xennet_interrupt(int irq, void *dev_id)
+static irqreturn_t xennet_tx_interrupt(int irq, void *dev_id)
 {
-	struct net_device *dev = dev_id;
-	struct netfront_info *np = netdev_priv(dev);
+	struct netfront_info *np = dev_id;
+	struct net_device *dev = np->netdev;
 	unsigned long flags;
 
 	spin_lock_irqsave(&np->tx_lock, flags);
+	xennet_tx_buf_gc(dev);
+	spin_unlock_irqrestore(&np->tx_lock, flags);
 
-	if (likely(netif_carrier_ok(dev))) {
-		xennet_tx_buf_gc(dev);
-		/* Under tx_lock: protects access to rx shared-ring indexes. */
-		if (RING_HAS_UNCONSUMED_RESPONSES(&np->rx))
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t xennet_rx_interrupt(int irq, void *dev_id)
+{
+	struct netfront_info *np = dev_id;
+	struct net_device *dev = np->netdev;
+
+	if (likely(netif_carrier_ok(dev) &&
+		   RING_HAS_UNCONSUMED_RESPONSES(&np->rx)))
 			napi_schedule(&np->napi);
-	}
 
-	spin_unlock_irqrestore(&np->tx_lock, flags);
+	return IRQ_HANDLED;
+}
 
+static irqreturn_t xennet_interrupt(int irq, void *dev_id)
+{
+	xennet_tx_interrupt(irq, dev_id);
+	xennet_rx_interrupt(irq, dev_id);
 	return IRQ_HANDLED;
 }
 
@@ -1451,9 +1471,14 @@ static void xennet_disconnect_backend(struct netfront_info *info)
 	spin_unlock_irq(&info->tx_lock);
 	spin_unlock_bh(&info->rx_lock);
 
-	if (info->netdev->irq)
-		unbind_from_irqhandler(info->netdev->irq, info->netdev);
-	info->evtchn = info->netdev->irq = 0;
+	if (info->tx_irq && (info->tx_irq == info->rx_irq))
+		unbind_from_irqhandler(info->tx_irq, info);
+	if (info->tx_irq && (info->tx_irq != info->rx_irq)) {
+		unbind_from_irqhandler(info->tx_irq, info);
+		unbind_from_irqhandler(info->rx_irq, info);
+	}
+	info->tx_evtchn = info->rx_evtchn = 0;
+	info->tx_irq = info->rx_irq = 0;
 
 	/* End access and free the pages */
 	xennet_end_access(info->tx_ring_ref, info->tx.sring);
@@ -1503,12 +1528,90 @@ static int xen_net_read_mac(struct xenbus_device *dev, u8 mac[])
 	return 0;
 }
 
+static int setup_netfront_single(struct netfront_info *info)
+{
+	int err;
+
+	err = xenbus_alloc_evtchn(info->xbdev, &info->tx_evtchn);
+	if (err < 0)
+		goto fail;
+
+	err = bind_evtchn_to_irqhandler(info->tx_evtchn,
+					xennet_interrupt,
+					0, info->netdev->name, info);
+	if (err < 0)
+		goto bind_fail;
+	info->rx_evtchn = info->tx_evtchn;
+	info->rx_irq = info->tx_irq = err;
+	dev_info(&info->xbdev->dev,
+		 "single event channel, evtchn = %d, irq = %d\n",
+		 info->tx_evtchn, info->tx_irq);
+
+	return 0;
+
+bind_fail:
+	xenbus_free_evtchn(info->xbdev, info->tx_evtchn);
+	info->tx_evtchn = 0;
+fail:
+	return err;
+}
+
+static int setup_netfront_split(struct netfront_info *info)
+{
+	int err;
+
+	err = xenbus_alloc_evtchn(info->xbdev, &info->tx_evtchn);
+	if (err < 0)
+		goto fail;
+	err = xenbus_alloc_evtchn(info->xbdev, &info->rx_evtchn);
+	if (err < 0)
+		goto alloc_rx_evtchn_fail;
+
+	snprintf(info->tx_irq_name, sizeof(info->tx_irq_name),
+		 "%s-tx", info->netdev->name);
+	err = bind_evtchn_to_irqhandler(info->tx_evtchn,
+					xennet_tx_interrupt,
+					0, info->tx_irq_name, info);
+	if (err < 0)
+		goto bind_tx_fail;
+	info->tx_irq = err;
+
+	snprintf(info->rx_irq_name, sizeof(info->rx_irq_name),
+		 "%s-rx", info->netdev->name);
+	err = bind_evtchn_to_irqhandler(info->rx_evtchn,
+					xennet_rx_interrupt,
+					0, info->rx_irq_name, info);
+	if (err < 0)
+		goto bind_rx_fail;
+	info->rx_irq = err;
+
+	dev_info(&info->xbdev->dev,
+		 "split event channels, tx_evtchn/irq = %d/%d, rx_evtchn/irq = %d/%d",
+		 info->tx_evtchn, info->tx_irq,
+		 info->rx_evtchn, info->rx_irq);
+
+	return 0;
+
+bind_rx_fail:
+	unbind_from_irqhandler(info->tx_irq, info);
+	info->tx_irq = 0;
+bind_tx_fail:
+	xenbus_free_evtchn(info->xbdev, info->rx_evtchn);
+	info->rx_evtchn = 0;
+alloc_rx_evtchn_fail:
+	xenbus_free_evtchn(info->xbdev, info->tx_evtchn);
+	info->tx_evtchn = 0;
+fail:
+	return err;
+}
+
 static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info)
 {
 	struct xen_netif_tx_sring *txs;
 	struct xen_netif_rx_sring *rxs;
 	int err;
 	struct net_device *netdev = info->netdev;
+	unsigned int feature_split_evtchn;
 
 	info->tx_ring_ref = GRANT_INVALID_REF;
 	info->rx_ring_ref = GRANT_INVALID_REF;
@@ -1516,6 +1619,12 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info)
 	info->tx.sring = NULL;
 	netdev->irq = 0;
 
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-split-event-channels", "%u",
+			   &feature_split_evtchn);
+	if (err < 0)
+		feature_split_evtchn = 0;
+
 	err = xen_net_read_mac(dev, netdev->dev_addr);
 	if (err) {
 		xenbus_dev_fatal(dev, err, "parsing %s/mac", dev->nodename);
@@ -1550,22 +1659,18 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info)
 		goto grant_rx_ring_fail;
 	info->rx_ring_ref = err;
 
-	err = xenbus_alloc_evtchn(dev, &info->evtchn);
+	if (feature_split_evtchn)
+		err = setup_netfront_split(info);
+	else
+		err = setup_netfront_single(info);
 	if (err)
 		goto alloc_evtchn_fail;
 
-	err = bind_evtchn_to_irqhandler(info->evtchn, xennet_interrupt,
-					0, netdev->name, netdev);
-	if (err < 0)
-		goto bind_fail;
-	netdev->irq = err;
 	return 0;
 
 	/* If we fail to setup netfront, it is safe to just revoke access to
 	 * granted pages because backend is not accessing it at this point.
 	 */
-bind_fail:
-	xenbus_free_evtchn(dev, info->evtchn);
 alloc_evtchn_fail:
 	gnttab_end_foreign_access_ref(info->rx_ring_ref, 0);
 grant_rx_ring_fail:
@@ -1610,11 +1715,27 @@ again:
 		message = "writing rx ring-ref";
 		goto abort_transaction;
 	}
-	err = xenbus_printf(xbt, dev->nodename,
-			    "event-channel", "%u", info->evtchn);
-	if (err) {
-		message = "writing event-channel";
-		goto abort_transaction;
+
+	if (info->tx_evtchn == info->rx_evtchn) {
+		err = xenbus_printf(xbt, dev->nodename,
+				    "event-channel", "%u", info->tx_evtchn);
+		if (err) {
+			message = "writing event-channel";
+			goto abort_transaction;
+		}
+	} else {
+		err = xenbus_printf(xbt, dev->nodename,
+				    "event-channel-tx", "%u", info->tx_evtchn);
+		if (err) {
+			message = "writing event-channel-tx";
+			goto abort_transaction;
+		}
+		err = xenbus_printf(xbt, dev->nodename,
+				    "event-channel-rx", "%u", info->rx_evtchn);
+		if (err) {
+			message = "writing event-channel-rx";
+			goto abort_transaction;
+		}
 	}
 
 	err = xenbus_printf(xbt, dev->nodename, "request-rx-copy", "%u",
@@ -1727,7 +1848,9 @@ static int xennet_connect(struct net_device *dev)
 	 * packets.
 	 */
 	netif_carrier_on(np->netdev);
-	notify_remote_via_irq(np->netdev->irq);
+	notify_remote_via_irq(np->tx_irq);
+	if (np->tx_irq != np->rx_irq)
+		notify_remote_via_irq(np->rx_irq);
 	xennet_tx_buf_gc(dev);
 	xennet_alloc_rx_buffers(dev);
 
-- 
1.7.10.4

[PATCH net-next 2/2] xen-netfront: split event channels feature support

[Wei Liu]

This patch adds a new feature called feature-split-event-channels for
netfront, enabling it to handle TX and RX events separately.

If netback does not support this feature, it falls back to use single event
channel.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 drivers/net/xen-netfront.c |  181 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 152 insertions(+), 29 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 5770e3b..716b2bd 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -85,7 +85,15 @@ struct netfront_info {
 
 	struct napi_struct napi;
 
-	unsigned int evtchn;
+	/* Split event channels support, tx_* == rx_* when using
+	 * single event channel.
+	 */
+	unsigned int tx_evtchn, rx_evtchn;
+	unsigned int tx_irq, rx_irq;
+	/* Only used when split event channels support is enabled */
+	char tx_irq_name[IFNAMSIZ+4]; /* DEVNAME-tx */
+	char rx_irq_name[IFNAMSIZ+4]; /* DEVNAME-rx */
+
 	struct xenbus_device *xbdev;
 
 	spinlock_t   tx_lock;
@@ -330,7 +338,7 @@ no_skb:
  push:
 	RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&np->rx, notify);
 	if (notify)
-		notify_remote_via_irq(np->netdev->irq);
+		notify_remote_via_irq(np->rx_irq);
 }
 
 static int xennet_open(struct net_device *dev)
@@ -623,7 +631,7 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&np->tx, notify);
 	if (notify)
-		notify_remote_via_irq(np->netdev->irq);
+		notify_remote_via_irq(np->tx_irq);
 
 	u64_stats_update_begin(&stats->syncp);
 	stats->tx_bytes += skb->len;
@@ -1254,23 +1262,35 @@ static int xennet_set_features(struct net_device *dev,
 	return 0;
 }
 
-static irqreturn_t xennet_interrupt(int irq, void *dev_id)
+static irqreturn_t xennet_tx_interrupt(int irq, void *dev_id)
 {
-	struct net_device *dev = dev_id;
-	struct netfront_info *np = netdev_priv(dev);
+	struct netfront_info *np = dev_id;
+	struct net_device *dev = np->netdev;
 	unsigned long flags;
 
 	spin_lock_irqsave(&np->tx_lock, flags);
+	xennet_tx_buf_gc(dev);
+	spin_unlock_irqrestore(&np->tx_lock, flags);
 
-	if (likely(netif_carrier_ok(dev))) {
-		xennet_tx_buf_gc(dev);
-		/* Under tx_lock: protects access to rx shared-ring indexes. */
-		if (RING_HAS_UNCONSUMED_RESPONSES(&np->rx))
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t xennet_rx_interrupt(int irq, void *dev_id)
+{
+	struct netfront_info *np = dev_id;
+	struct net_device *dev = np->netdev;
+
+	if (likely(netif_carrier_ok(dev) &&
+		   RING_HAS_UNCONSUMED_RESPONSES(&np->rx)))
 			napi_schedule(&np->napi);
-	}
 
-	spin_unlock_irqrestore(&np->tx_lock, flags);
+	return IRQ_HANDLED;
+}
 
+static irqreturn_t xennet_interrupt(int irq, void *dev_id)
+{
+	xennet_tx_interrupt(irq, dev_id);
+	xennet_rx_interrupt(irq, dev_id);
 	return IRQ_HANDLED;
 }
 
@@ -1451,9 +1471,14 @@ static void xennet_disconnect_backend(struct netfront_info *info)
 	spin_unlock_irq(&info->tx_lock);
 	spin_unlock_bh(&info->rx_lock);
 
-	if (info->netdev->irq)
-		unbind_from_irqhandler(info->netdev->irq, info->netdev);
-	info->evtchn = info->netdev->irq = 0;
+	if (info->tx_irq && (info->tx_irq == info->rx_irq))
+		unbind_from_irqhandler(info->tx_irq, info);
+	if (info->tx_irq && (info->tx_irq != info->rx_irq)) {
+		unbind_from_irqhandler(info->tx_irq, info);
+		unbind_from_irqhandler(info->rx_irq, info);
+	}
+	info->tx_evtchn = info->rx_evtchn = 0;
+	info->tx_irq = info->rx_irq = 0;
 
 	/* End access and free the pages */
 	xennet_end_access(info->tx_ring_ref, info->tx.sring);
@@ -1503,12 +1528,90 @@ static int xen_net_read_mac(struct xenbus_device *dev, u8 mac[])
 	return 0;
 }
 
+static int setup_netfront_single(struct netfront_info *info)
+{
+	int err;
+
+	err = xenbus_alloc_evtchn(info->xbdev, &info->tx_evtchn);
+	if (err < 0)
+		goto fail;
+
+	err = bind_evtchn_to_irqhandler(info->tx_evtchn,
+					xennet_interrupt,
+					0, info->netdev->name, info);
+	if (err < 0)
+		goto bind_fail;
+	info->rx_evtchn = info->tx_evtchn;
+	info->rx_irq = info->tx_irq = err;
+	dev_info(&info->xbdev->dev,
+		 "single event channel, evtchn = %d, irq = %d\n",
+		 info->tx_evtchn, info->tx_irq);
+
+	return 0;
+
+bind_fail:
+	xenbus_free_evtchn(info->xbdev, info->tx_evtchn);
+	info->tx_evtchn = 0;
+fail:
+	return err;
+}
+
+static int setup_netfront_split(struct netfront_info *info)
+{
+	int err;
+
+	err = xenbus_alloc_evtchn(info->xbdev, &info->tx_evtchn);
+	if (err < 0)
+		goto fail;
+	err = xenbus_alloc_evtchn(info->xbdev, &info->rx_evtchn);
+	if (err < 0)
+		goto alloc_rx_evtchn_fail;
+
+	snprintf(info->tx_irq_name, sizeof(info->tx_irq_name),
+		 "%s-tx", info->netdev->name);
+	err = bind_evtchn_to_irqhandler(info->tx_evtchn,
+					xennet_tx_interrupt,
+					0, info->tx_irq_name, info);
+	if (err < 0)
+		goto bind_tx_fail;
+	info->tx_irq = err;
+
+	snprintf(info->rx_irq_name, sizeof(info->rx_irq_name),
+		 "%s-rx", info->netdev->name);
+	err = bind_evtchn_to_irqhandler(info->rx_evtchn,
+					xennet_rx_interrupt,
+					0, info->rx_irq_name, info);
+	if (err < 0)
+		goto bind_rx_fail;
+	info->rx_irq = err;
+
+	dev_info(&info->xbdev->dev,
+		 "split event channels, tx_evtchn/irq = %d/%d, rx_evtchn/irq = %d/%d",
+		 info->tx_evtchn, info->tx_irq,
+		 info->rx_evtchn, info->rx_irq);
+
+	return 0;
+
+bind_rx_fail:
+	unbind_from_irqhandler(info->tx_irq, info);
+	info->tx_irq = 0;
+bind_tx_fail:
+	xenbus_free_evtchn(info->xbdev, info->rx_evtchn);
+	info->rx_evtchn = 0;
+alloc_rx_evtchn_fail:
+	xenbus_free_evtchn(info->xbdev, info->tx_evtchn);
+	info->tx_evtchn = 0;
+fail:
+	return err;
+}
+
 static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info)
 {
 	struct xen_netif_tx_sring *txs;
 	struct xen_netif_rx_sring *rxs;
 	int err;
 	struct net_device *netdev = info->netdev;
+	unsigned int feature_split_evtchn;
 
 	info->tx_ring_ref = GRANT_INVALID_REF;
 	info->rx_ring_ref = GRANT_INVALID_REF;
@@ -1516,6 +1619,12 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info)
 	info->tx.sring = NULL;
 	netdev->irq = 0;
 
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-split-event-channels", "%u",
+			   &feature_split_evtchn);
+	if (err < 0)
+		feature_split_evtchn = 0;
+
 	err = xen_net_read_mac(dev, netdev->dev_addr);
 	if (err) {
 		xenbus_dev_fatal(dev, err, "parsing %s/mac", dev->nodename);
@@ -1550,22 +1659,18 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info)
 		goto grant_rx_ring_fail;
 	info->rx_ring_ref = err;
 
-	err = xenbus_alloc_evtchn(dev, &info->evtchn);
+	if (feature_split_evtchn)
+		err = setup_netfront_split(info);
+	else
+		err = setup_netfront_single(info);
 	if (err)
 		goto alloc_evtchn_fail;
 
-	err = bind_evtchn_to_irqhandler(info->evtchn, xennet_interrupt,
-					0, netdev->name, netdev);
-	if (err < 0)
-		goto bind_fail;
-	netdev->irq = err;
 	return 0;
 
 	/* If we fail to setup netfront, it is safe to just revoke access to
 	 * granted pages because backend is not accessing it at this point.
 	 */
-bind_fail:
-	xenbus_free_evtchn(dev, info->evtchn);
 alloc_evtchn_fail:
 	gnttab_end_foreign_access_ref(info->rx_ring_ref, 0);
 grant_rx_ring_fail:
@@ -1610,11 +1715,27 @@ again:
 		message = "writing rx ring-ref";
 		goto abort_transaction;
 	}
-	err = xenbus_printf(xbt, dev->nodename,
-			    "event-channel", "%u", info->evtchn);
-	if (err) {
-		message = "writing event-channel";
-		goto abort_transaction;
+
+	if (info->tx_evtchn == info->rx_evtchn) {
+		err = xenbus_printf(xbt, dev->nodename,
+				    "event-channel", "%u", info->tx_evtchn);
+		if (err) {
+			message = "writing event-channel";
+			goto abort_transaction;
+		}
+	} else {
+		err = xenbus_printf(xbt, dev->nodename,
+				    "event-channel-tx", "%u", info->tx_evtchn);
+		if (err) {
+			message = "writing event-channel-tx";
+			goto abort_transaction;
+		}
+		err = xenbus_printf(xbt, dev->nodename,
+				    "event-channel-rx", "%u", info->rx_evtchn);
+		if (err) {
+			message = "writing event-channel-rx";
+			goto abort_transaction;
+		}
 	}
 
 	err = xenbus_printf(xbt, dev->nodename, "request-rx-copy", "%u",
@@ -1727,7 +1848,9 @@ static int xennet_connect(struct net_device *dev)
 	 * packets.
 	 */
 	netif_carrier_on(np->netdev);
-	notify_remote_via_irq(np->netdev->irq);
+	notify_remote_via_irq(np->tx_irq);
+	if (np->tx_irq != np->rx_irq)
+		notify_remote_via_irq(np->rx_irq);
 	xennet_tx_buf_gc(dev);
 	xennet_alloc_rx_buffers(dev);
 
-- 
1.7.10.4

Re: [Xen-devel] [PATCH net-next 2/2] xen-netfront: split event channels feature support

[David Vrabel]

On 21/05/13 00:02, Wei Liu wrote:
> This patch adds a new feature called feature-split-event-channels for
> netfront, enabling it to handle TX and RX events separately.
> 
> If netback does not support this feature, it falls back to use single event
> channel.
[...]
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -1503,12 +1528,90 @@ static int xen_net_read_mac(struct xenbus_device *dev, u8 mac[])
>  	return 0;
>  }
>  
> +static int setup_netfront_single(struct netfront_info *info)
> +{
> +	int err;
> +
> +	err = xenbus_alloc_evtchn(info->xbdev, &info->tx_evtchn);
> +	if (err < 0)
> +		goto fail;
> +
> +	err = bind_evtchn_to_irqhandler(info->tx_evtchn,
> +					xennet_interrupt,
> +					0, info->netdev->name, info);
> +	if (err < 0)
> +		goto bind_fail;
> +	info->rx_evtchn = info->tx_evtchn;
> +	info->rx_irq = info->tx_irq = err;
> +	dev_info(&info->xbdev->dev,
> +		 "single event channel, evtchn = %d, irq = %d\n",
> +		 info->tx_evtchn, info->tx_irq);

This message is pointless chatter as the information is available
elsewhere.  Please remove.

> +	dev_info(&info->xbdev->dev,
> +		 "split event channels, tx_evtchn/irq = %d/%d, rx_evtchn/irq = %d/%d",
> +		 info->tx_evtchn, info->tx_irq,
> +		 info->rx_evtchn, info->rx_irq);

Similarly.

Otherwise,

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

Re: [Xen-devel] [PATCH net-next 2/2] xen-netfront: split event channels feature support

[Wei Liu]

On Tue, May 21, 2013 at 02:39:01PM +0100, David Vrabel wrote:
[...]
> > +	info->rx_evtchn = info->tx_evtchn;
> > +	info->rx_irq = info->tx_irq = err;
> > +	dev_info(&info->xbdev->dev,
> > +		 "single event channel, evtchn = %d, irq = %d\n",
> > +		 info->tx_evtchn, info->tx_irq);
> 
> This message is pointless chatter as the information is available
> elsewhere.  Please remove.
> 

Do you mean /proc/interrupt by "elsewhere"? Information there doesn't
give you irq<->evtchn mapping info.


Wei.

Re: [PATCH net-next 2/2] xen-netfront: split event channels feature support

[Konrad Rzeszutek Wilk]

On Tue, May 21, 2013 at 02:41:49PM +0100, Wei Liu wrote:
> On Tue, May 21, 2013 at 02:39:01PM +0100, David Vrabel wrote:
> [...]
> > > +	info->rx_evtchn = info->tx_evtchn;
> > > +	info->rx_irq = info->tx_irq = err;
> > > +	dev_info(&info->xbdev->dev,
> > > +		 "single event channel, evtchn = %d, irq = %d\n",
> > > +		 info->tx_evtchn, info->tx_irq);
> > 
> > This message is pointless chatter as the information is available
> > elsewhere.  Please remove.
> > 
> 
> Do you mean /proc/interrupt by "elsewhere"? Information there doesn't
> give you irq<->evtchn mapping info.

If you really want to, that should be implemented via an DebugFS entry that can
be driven from drivers/xen/events.c. As the events.c is the generic code that sets
up the Xen events, pirqs, ipis, <-> Linux IRQs.

Re: [Xen-devel] [PATCH net-next 2/2] xen-netfront: split event channels feature support

[Konrad Rzeszutek Wilk]

On Tue, May 21, 2013 at 02:41:49PM +0100, Wei Liu wrote:
> On Tue, May 21, 2013 at 02:39:01PM +0100, David Vrabel wrote:
> [...]
> > > +	info->rx_evtchn = info->tx_evtchn;
> > > +	info->rx_irq = info->tx_irq = err;
> > > +	dev_info(&info->xbdev->dev,
> > > +		 "single event channel, evtchn = %d, irq = %d\n",
> > > +		 info->tx_evtchn, info->tx_irq);
> > 
> > This message is pointless chatter as the information is available
> > elsewhere.  Please remove.
> > 
> 
> Do you mean /proc/interrupt by "elsewhere"? Information there doesn't
> give you irq<->evtchn mapping info.

If you really want to, that should be implemented via an DebugFS entry that can
be driven from drivers/xen/events.c. As the events.c is the generic code that sets
up the Xen events, pirqs, ipis, <-> Linux IRQs.

Re: [Xen-devel] [PATCH net-next 2/2] xen-netfront: split event channels feature support

[Wei Liu]

On Tue, May 21, 2013 at 12:28:25PM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, May 21, 2013 at 02:41:49PM +0100, Wei Liu wrote:
> > On Tue, May 21, 2013 at 02:39:01PM +0100, David Vrabel wrote:
> > [...]
> > > > +	info->rx_evtchn = info->tx_evtchn;
> > > > +	info->rx_irq = info->tx_irq = err;
> > > > +	dev_info(&info->xbdev->dev,
> > > > +		 "single event channel, evtchn = %d, irq = %d\n",
> > > > +		 info->tx_evtchn, info->tx_irq);
> > > 
> > > This message is pointless chatter as the information is available
> > > elsewhere.  Please remove.
> > > 
> > 
> > Do you mean /proc/interrupt by "elsewhere"? Information there doesn't
> > give you irq<->evtchn mapping info.
> 
> If you really want to, that should be implemented via an DebugFS entry that can
> be driven from drivers/xen/events.c. As the events.c is the generic code that sets
> up the Xen events, pirqs, ipis, <-> Linux IRQs.

I mainly focus on upstreaming all necessary features for a new Xen
network baseline, so I would rather not touch other files.

I will delete these dev_info, they don't seem to be critical anyway.


Wei.

Re: [PATCH net-next 2/2] xen-netfront: split event channels feature support

[Wei Liu]

On Tue, May 21, 2013 at 12:28:25PM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, May 21, 2013 at 02:41:49PM +0100, Wei Liu wrote:
> > On Tue, May 21, 2013 at 02:39:01PM +0100, David Vrabel wrote:
> > [...]
> > > > +	info->rx_evtchn = info->tx_evtchn;
> > > > +	info->rx_irq = info->tx_irq = err;
> > > > +	dev_info(&info->xbdev->dev,
> > > > +		 "single event channel, evtchn = %d, irq = %d\n",
> > > > +		 info->tx_evtchn, info->tx_irq);
> > > 
> > > This message is pointless chatter as the information is available
> > > elsewhere.  Please remove.
> > > 
> > 
> > Do you mean /proc/interrupt by "elsewhere"? Information there doesn't
> > give you irq<->evtchn mapping info.
> 
> If you really want to, that should be implemented via an DebugFS entry that can
> be driven from drivers/xen/events.c. As the events.c is the generic code that sets
> up the Xen events, pirqs, ipis, <-> Linux IRQs.

I mainly focus on upstreaming all necessary features for a new Xen
network baseline, so I would rather not touch other files.

I will delete these dev_info, they don't seem to be critical anyway.


Wei.

Re: [Xen-devel] [PATCH net-next 2/2] xen-netfront: split event channels feature support

[David Vrabel]

On 21/05/13 14:41, Wei Liu wrote:
> On Tue, May 21, 2013 at 02:39:01PM +0100, David Vrabel wrote:
> [...]
>>> +	info->rx_evtchn = info->tx_evtchn;
>>> +	info->rx_irq = info->tx_irq = err;
>>> +	dev_info(&info->xbdev->dev,
>>> +		 "single event channel, evtchn = %d, irq = %d\n",
>>> +		 info->tx_evtchn, info->tx_irq);
>>
>> This message is pointless chatter as the information is available
>> elsewhere.  Please remove.
>>
> 
> Do you mean /proc/interrupt by "elsewhere"? Information there doesn't
> give you irq<->evtchn mapping info.

A combination of /proc/interrupts and the xenstore keys.

David

Re: [PATCH net-next 2/2] xen-netfront: split event channels feature support

[David Vrabel]

On 21/05/13 14:41, Wei Liu wrote:
> On Tue, May 21, 2013 at 02:39:01PM +0100, David Vrabel wrote:
> [...]
>>> +	info->rx_evtchn = info->tx_evtchn;
>>> +	info->rx_irq = info->tx_irq = err;
>>> +	dev_info(&info->xbdev->dev,
>>> +		 "single event channel, evtchn = %d, irq = %d\n",
>>> +		 info->tx_evtchn, info->tx_irq);
>>
>> This message is pointless chatter as the information is available
>> elsewhere.  Please remove.
>>
> 
> Do you mean /proc/interrupt by "elsewhere"? Information there doesn't
> give you irq<->evtchn mapping info.

A combination of /proc/interrupts and the xenstore keys.

David

Re: [PATCH net-next 2/2] xen-netfront: split event channels feature support

[Wei Liu]

On Tue, May 21, 2013 at 02:39:01PM +0100, David Vrabel wrote:
[...]
> > +	info->rx_evtchn = info->tx_evtchn;
> > +	info->rx_irq = info->tx_irq = err;
> > +	dev_info(&info->xbdev->dev,
> > +		 "single event channel, evtchn = %d, irq = %d\n",
> > +		 info->tx_evtchn, info->tx_irq);
> 
> This message is pointless chatter as the information is available
> elsewhere.  Please remove.
> 

Do you mean /proc/interrupt by "elsewhere"? Information there doesn't
give you irq<->evtchn mapping info.


Wei.

Re: [PATCH net-next 2/2] xen-netfront: split event channels feature support

[David Vrabel]

On 21/05/13 00:02, Wei Liu wrote:
> This patch adds a new feature called feature-split-event-channels for
> netfront, enabling it to handle TX and RX events separately.
> 
> If netback does not support this feature, it falls back to use single event
> channel.
[...]
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -1503,12 +1528,90 @@ static int xen_net_read_mac(struct xenbus_device *dev, u8 mac[])
>  	return 0;
>  }
>  
> +static int setup_netfront_single(struct netfront_info *info)
> +{
> +	int err;
> +
> +	err = xenbus_alloc_evtchn(info->xbdev, &info->tx_evtchn);
> +	if (err < 0)
> +		goto fail;
> +
> +	err = bind_evtchn_to_irqhandler(info->tx_evtchn,
> +					xennet_interrupt,
> +					0, info->netdev->name, info);
> +	if (err < 0)
> +		goto bind_fail;
> +	info->rx_evtchn = info->tx_evtchn;
> +	info->rx_irq = info->tx_irq = err;
> +	dev_info(&info->xbdev->dev,
> +		 "single event channel, evtchn = %d, irq = %d\n",
> +		 info->tx_evtchn, info->tx_irq);

This message is pointless chatter as the information is available
elsewhere.  Please remove.

> +	dev_info(&info->xbdev->dev,
> +		 "split event channels, tx_evtchn/irq = %d/%d, rx_evtchn/irq = %d/%d",
> +		 info->tx_evtchn, info->tx_irq,
> +		 info->rx_evtchn, info->rx_irq);

Similarly.

Otherwise,

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

Re: [PATCH net-next 0/2] Xen network: split event channels support

[David Miller]

If you use the same exact subject line in all of your patches in the
series, nobody reading the shortlog can tell what's different about
them.

You absolutely must resubmit this with more descriptive subject lines
which more accurately describe what each patch uniquely does.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[David Miller]

If you use the same exact subject line in all of your patches in the
series, nobody reading the shortlog can tell what's different about
them.

You absolutely must resubmit this with more descriptive subject lines
which more accurately describe what each patch uniquely does.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[Wei Liu]

On Mon, May 20, 2013 at 11:27:52PM -0700, David Miller wrote:
> 
> If you use the same exact subject line in all of your patches in the
> series, nobody reading the shortlog can tell what's different about
> them.
> 
> You absolutely must resubmit this with more descriptive subject lines
> which more accurately describe what each patch uniquely does.

I'm not sure I get what you mean. Their subject lines look alike, but
not exactly the same.

So this patch set adds a new feature to Xen network device, which is
called split event channels. The subject lines of two patches, one for
frontend and one for backend, speak for themselves.

No matter how I change the subject lines, they will probably still look
alike except for the leading "xen-netfront / xen-netback".  I can squash
them into one changeset if that better suits your workflow, but it is
better to leave them separate IMHO so that we can test them separately.

Advice welcomed.

Thanks
Wei.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[David Miller]

From: Wei Liu <wei.liu2@citrix.com>
Date: Tue, 21 May 2013 09:00:40 +0100

> I'm not sure I get what you mean. Their subject lines look alike, but
> not exactly the same.

I see, the prefix is different, sorry about that.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[David Miller]

From: Wei Liu <wei.liu2@citrix.com>
Date: Tue, 21 May 2013 09:00:40 +0100

> I'm not sure I get what you mean. Their subject lines look alike, but
> not exactly the same.

I see, the prefix is different, sorry about that.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[Ian Campbell]

On Tue, 2013-05-21 at 09:00 +0100, Wei Liu wrote:
> On Mon, May 20, 2013 at 11:27:52PM -0700, David Miller wrote:
> > 
> > If you use the same exact subject line in all of your patches in the
> > series, nobody reading the shortlog can tell what's different about
> > them.
> > 
> > You absolutely must resubmit this with more descriptive subject lines
> > which more accurately describe what each patch uniquely does.
> 
> I'm not sure I get what you mean. Their subject lines look alike, but
> not exactly the same.
> 
> So this patch set adds a new feature to Xen network device, which is
> called split event channels. The subject lines of two patches, one for
> frontend and one for backend, speak for themselves.

This is true, but I wonder if peoples eyes tend to skip over the initial
prefix.

> No matter how I change the subject lines, they will probably still look
> alike except for the leading "xen-netfront / xen-netback".  I can squash
> them into one changeset if that better suits your workflow, but it is
> better to leave them separate IMHO so that we can test them separately.
>
> Advice welcomed.

How about "xen-netback: Split event channel support for Xen Backend
driver" and the equivalent for frontend? 

I'd prefer to keep it as two changesets if possible.

Ian.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[Wei Liu]

On Tue, May 21, 2013 at 09:17:35AM +0100, Ian Campbell wrote:
[...]
> 
> How about "xen-netback: Split event channel support for Xen Backend
> driver" and the equivalent for frontend? 
> 

Sure, this can avoid future confusion.


Wei.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[Wei Liu]

On Tue, May 21, 2013 at 09:17:35AM +0100, Ian Campbell wrote:
[...]
> 
> How about "xen-netback: Split event channel support for Xen Backend
> driver" and the equivalent for frontend? 
> 

Sure, this can avoid future confusion.


Wei.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[Ian Campbell]

On Tue, 2013-05-21 at 09:00 +0100, Wei Liu wrote:
> On Mon, May 20, 2013 at 11:27:52PM -0700, David Miller wrote:
> > 
> > If you use the same exact subject line in all of your patches in the
> > series, nobody reading the shortlog can tell what's different about
> > them.
> > 
> > You absolutely must resubmit this with more descriptive subject lines
> > which more accurately describe what each patch uniquely does.
> 
> I'm not sure I get what you mean. Their subject lines look alike, but
> not exactly the same.
> 
> So this patch set adds a new feature to Xen network device, which is
> called split event channels. The subject lines of two patches, one for
> frontend and one for backend, speak for themselves.

This is true, but I wonder if peoples eyes tend to skip over the initial
prefix.

> No matter how I change the subject lines, they will probably still look
> alike except for the leading "xen-netfront / xen-netback".  I can squash
> them into one changeset if that better suits your workflow, but it is
> better to leave them separate IMHO so that we can test them separately.
>
> Advice welcomed.

How about "xen-netback: Split event channel support for Xen Backend
driver" and the equivalent for frontend? 

I'd prefer to keep it as two changesets if possible.

Ian.

Re: [PATCH net-next 0/2] Xen network: split event channels support

[Wei Liu]

On Mon, May 20, 2013 at 11:27:52PM -0700, David Miller wrote:
> 
> If you use the same exact subject line in all of your patches in the
> series, nobody reading the shortlog can tell what's different about
> them.
> 
> You absolutely must resubmit this with more descriptive subject lines
> which more accurately describe what each patch uniquely does.

I'm not sure I get what you mean. Their subject lines look alike, but
not exactly the same.

So this patch set adds a new feature to Xen network device, which is
called split event channels. The subject lines of two patches, one for
frontend and one for backend, speak for themselves.

No matter how I change the subject lines, they will probably still look
alike except for the leading "xen-netfront / xen-netback".  I can squash
them into one changeset if that better suits your workflow, but it is
better to leave them separate IMHO so that we can test them separately.

Advice welcomed.

Thanks
Wei.