| Message ID | 1357576074-24245-1-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jan 07, 2013 at 05:27:54PM +0100, Thomas Petazzoni wrote: > From: Dmitri Epshtein <dima@marvell.com> > > In order for the driver to behave properly in a SMP context, the same > transmit queue should be used by the kernel in dev_queue_xmit() and in > the driver's mvneta_tx() function. To achieve that, the driver now > implements the ->ndo_select_txq() operation. > > For now, it always returns the same transmit queue, txq_def, until the > driver is expanded to properly take advantage of the multiqueue > capabilities of the hardware. > > Without this patch, the network driver crashes the kernel almost > immediately on Armada XP platforms, if the network load is at least a > little bit parallel (i.e several threads). > > [Thomas Petazzoni: reword commit message] > Signed-off-by: Dmitri Epshtein <dima@marvell.com> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Acked-by: Jason Cooper <jason@lakedaemon.net> I'll assume this is going through -net, let me know if it should be otherwise. thx, Jason.
On Mon, 2013-01-07 at 17:27 +0100, Thomas Petazzoni wrote: > From: Dmitri Epshtein <dima@marvell.com> > > In order for the driver to behave properly in a SMP context, the same > transmit queue should be used by the kernel in dev_queue_xmit() and in > the driver's mvneta_tx() function. To achieve that, the driver now > implements the ->ndo_select_txq() operation. > > For now, it always returns the same transmit queue, txq_def, until the > driver is expanded to properly take advantage of the multiqueue > capabilities of the hardware. [...] Well it looks like the driver already sets up multiple TX queues, and you just need to call skb_get_queue_mapping(skb) to look up the TX queue in mvneta_tx(). Also since the numbers of queues are variable you should be calling alloc_etherdev_mqs(sizeof(struct mvneta_port), txq_number, rxq_number) instead of alloc_etherdev_mq(..., 8) in mvneta_probe(). If for some reason the current hardware initialisation doesn't actually work for more than 1 TX queue then change the number you pass to alloc_etherdev_mqs(). Ben.
On 01/07/2013 05:27 PM, Thomas Petazzoni wrote: > From: Dmitri Epshtein <dima@marvell.com> > > In order for the driver to behave properly in a SMP context, the same > transmit queue should be used by the kernel in dev_queue_xmit() and in > the driver's mvneta_tx() function. To achieve that, the driver now > implements the ->ndo_select_txq() operation. > > For now, it always returns the same transmit queue, txq_def, until the > driver is expanded to properly take advantage of the multiqueue > capabilities of the hardware. > > Without this patch, the network driver crashes the kernel almost > immediately on Armada XP platforms, if the network load is at least a > little bit parallel (i.e several threads). > > [Thomas Petazzoni: reword commit message] > Signed-off-by: Dmitri Epshtein <dima@marvell.com> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> I have tested this patch on my Armada XP DB board and on the OpenBlocks AX3 board. I confirmed that it fixes the problem when multiple thread are used. You can add my Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com> and also my Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Or a combination of the two if you prefer! > --- > This is 3.8-rc material. > --- > drivers/net/ethernet/marvell/mvneta.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c > index b6025c3..af2c421 100644 > --- a/drivers/net/ethernet/marvell/mvneta.c > +++ b/drivers/net/ethernet/marvell/mvneta.c > @@ -1310,6 +1310,17 @@ static u32 mvneta_skb_tx_csum(struct mvneta_port *pp, struct sk_buff *skb) > return MVNETA_TX_L4_CSUM_NOT; > } > > +static u16 mvneta_tx_policy(struct mvneta_port *pp, struct sk_buff *skb) > +{ > + return (u16)txq_def; > +} > + > +static u16 mvneta_select_txq(struct net_device *dev, struct sk_buff *skb) > +{ > + struct mvneta_port *pp = netdev_priv(dev); > + return mvneta_tx_policy(pp, skb); > +} > + > /* Returns rx queue pointer (find last set bit) according to causeRxTx > * value > */ > @@ -1476,7 +1487,8 @@ error: > static int mvneta_tx(struct sk_buff *skb, struct net_device *dev) > { > struct mvneta_port *pp = netdev_priv(dev); > - struct mvneta_tx_queue *txq = &pp->txqs[txq_def]; > + u16 txq_id = mvneta_tx_policy(pp, skb); > + struct mvneta_tx_queue *txq = &pp->txqs[txq_id]; > struct mvneta_tx_desc *tx_desc; > struct netdev_queue *nq; > int frags = 0; > @@ -1486,7 +1498,7 @@ static int mvneta_tx(struct sk_buff *skb, struct net_device *dev) > goto out; > > frags = skb_shinfo(skb)->nr_frags + 1; > - nq = netdev_get_tx_queue(dev, txq_def); > + nq = netdev_get_tx_queue(dev, txq_id); > > /* Get a descriptor for the first part of the packet */ > tx_desc = mvneta_txq_next_desc_get(txq); > @@ -2550,6 +2562,7 @@ static const struct net_device_ops mvneta_netdev_ops = { > .ndo_change_mtu = mvneta_change_mtu, > .ndo_tx_timeout = mvneta_tx_timeout, > .ndo_get_stats64 = mvneta_get_stats64, > + .ndo_select_queue = mvneta_select_txq, > }; > > const struct ethtool_ops mvneta_eth_tool_ops = { >
From: Jason Cooper <jason@lakedaemon.net> Date: Mon, 7 Jan 2013 11:44:15 -0500 > I'll assume this is going through -net, let me know if it should be > otherwise. It needs to be fixed up before it can be applied anywhere, see Ben Hutchings's feedback.
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index b6025c3..af2c421 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -1310,6 +1310,17 @@ static u32 mvneta_skb_tx_csum(struct mvneta_port *pp, struct sk_buff *skb) return MVNETA_TX_L4_CSUM_NOT; } +static u16 mvneta_tx_policy(struct mvneta_port *pp, struct sk_buff *skb) +{ + return (u16)txq_def; +} + +static u16 mvneta_select_txq(struct net_device *dev, struct sk_buff *skb) +{ + struct mvneta_port *pp = netdev_priv(dev); + return mvneta_tx_policy(pp, skb); +} + /* Returns rx queue pointer (find last set bit) according to causeRxTx * value */ @@ -1476,7 +1487,8 @@ error: static int mvneta_tx(struct sk_buff *skb, struct net_device *dev) { struct mvneta_port *pp = netdev_priv(dev); - struct mvneta_tx_queue *txq = &pp->txqs[txq_def]; + u16 txq_id = mvneta_tx_policy(pp, skb); + struct mvneta_tx_queue *txq = &pp->txqs[txq_id]; struct mvneta_tx_desc *tx_desc; struct netdev_queue *nq; int frags = 0; @@ -1486,7 +1498,7 @@ static int mvneta_tx(struct sk_buff *skb, struct net_device *dev) goto out; frags = skb_shinfo(skb)->nr_frags + 1; - nq = netdev_get_tx_queue(dev, txq_def); + nq = netdev_get_tx_queue(dev, txq_id); /* Get a descriptor for the first part of the packet */ tx_desc = mvneta_txq_next_desc_get(txq); @@ -2550,6 +2562,7 @@ static const struct net_device_ops mvneta_netdev_ops = { .ndo_change_mtu = mvneta_change_mtu, .ndo_tx_timeout = mvneta_tx_timeout, .ndo_get_stats64 = mvneta_get_stats64, + .ndo_select_queue = mvneta_select_txq, }; const struct ethtool_ops mvneta_eth_tool_ops = {