From: Stephen Hemminger <shemminger@vyatta•com>
To: Bart Van Assche <bvanassche@acm•org>
Cc: Francois Romieu <romieu@fr•zoreil.com>,
netdev@vger•kernel.org, Stephen Hemminger <shemminger@osdl•org>,
Jeff Garzik <jeff@garzik•org>
Subject: Re: [PATCH, RFC] skge: Fix race in tx path
Date: Sun, 4 Mar 2012 21:25:07 -0800 [thread overview]
Message-ID: <20120304212507.6d910d99@nehalam.linuxnetplumber.net> (raw)
In-Reply-To: <3669580.gQc5FCr96Z@asus>
On Mon, 27 Feb 2012 19:18:58 +0000
Bart Van Assche <bvanassche@acm•org> wrote:
> > With this change it seems possible for skge_tx_done to unmap a buffer
> > before it is sent when the requests for skge_xmit_frame are close enough.
>
> Thanks for the feedback. Does the patch below look better ?
>
> ---
> drivers/net/ethernet/marvell/skge.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c
> index 33947ac..55723b5 100644
> --- a/drivers/net/ethernet/marvell/skge.c
> +++ b/drivers/net/ethernet/marvell/skge.c
> @@ -2800,15 +2800,15 @@ static netdev_tx_t skge_xmit_frame(struct sk_buff *skb,
>
> netdev_sent_queue(dev, skb->len);
>
> + skge->tx_ring.to_use = e->next;
> + smp_wmb();
> +
> skge_write8(hw, Q_ADDR(txqaddr[skge->port], Q_CSR), CSR_START);
>
> netif_printk(skge, tx_queued, KERN_DEBUG, skge->netdev,
> "tx queued, slot %td, len %d\n",
> e - skge->tx_ring.start, skb->len);
>
> - skge->tx_ring.to_use = e->next;
> - smp_wmb();
> -
> if (skge_avail(&skge->tx_ring) <= TX_LOW_WATER) {
> netdev_dbg(dev, "transmit queue full\n");
> netif_stop_queue(dev);
I don't see how the problem you are seeing.
If the race happens with old code:
ring element is setup and started.
IRQ happens
........ skge_tx_done()
starts cleaning
stops prematurely
ring_to_use is updated
one skb stays stuck in ring until next TX clean
So the race would just leave one skb behind (until next transmit).
Looking at tg3, you will see similar code.
More likely mmiowb() is needd at end of tx.
prev parent reply other threads:[~2012-03-05 5:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-24 19:33 [PATCH, RFC] skge: Fix race in tx path Bart Van Assche
2012-02-26 20:27 ` Francois Romieu
2012-02-27 19:18 ` Bart Van Assche
2012-02-27 20:52 ` Francois Romieu
2012-03-05 5:25 ` Stephen Hemminger [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120304212507.6d910d99@nehalam.linuxnetplumber.net \
--to=shemminger@vyatta$(echo .)com \
--cc=bvanassche@acm$(echo .)org \
--cc=jeff@garzik$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=romieu@fr$(echo .)zoreil.com \
--cc=shemminger@osdl$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox