All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
To: David L Stevens <david.stevens@oracle.com>
Cc: David Miller <davem@davemloft.net>, netdev@vger.kernel.org
Subject: Re: [PATCH net-next] sunvnet: improve error handling when a remote crashes
Date: Mon, 26 Jan 2015 14:54:03 -0500	[thread overview]
Message-ID: <20150126195403.GE6437@oracle.com> (raw)
In-Reply-To: <54C6911B.5040501@oracle.com>




> @@ -934,36 +933,36 @@ static struct sk_buff *vnet_clean_tx_ring(struct vnet_port *port,
>  
>  	*pending = 0;
>  
> -	txi = dr->prod-1;
> -	if (txi < 0)
> -		txi = VNET_TX_RING_SIZE-1;
> -
> +	txi = dr->prod;

As I understand it, this starts at dr->prod and goes through all
descriptors, cleaning up !READY descriptors as it goes around.

I think you'll have a higher reclaim rate for finding !READY if you
started at dr->cons instead: dr->cons is the one that was last ACK'ed,
and that ack would only have been sent after the peer had marked the 
descriptor as DONE. (consumer would have had a chance to read more
descriptors, by the time the tx-reclaim loop goes around) 

> +		if (port->tx_bufs[txi].skb) {
> +			if (d->hdr.state != VIO_DESC_DONE)
> +				pr_warn("invalid ring buffer state %d\n",
> +					d->hdr.state);

I would even suggest skipping the pr_warn (maybe make it a viodbg
instead) as it might alarm the end-user (who cannot really do 
anything about it other than call us anyway :-)).

>  			   dr->cookies, dr->ncookies);
> +	if (active_freed)
> +		pr_warn("%s: active transmit buffers freed for remote %pM\n",
> +			dev->name, port->raddr);

Same comment as above.

In general, I think we need some sysfs/ethtool  bean-counters/statistics
for sunvnet, to keep track of this sort of thing efficiently in 
a production env without triggering red-herrings calls.

--Sowmini

  parent reply	other threads:[~2015-01-26 19:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-26 19:10 [PATCH net-next] sunvnet: improve error handling when a remote crashes David L Stevens
2015-01-26 19:48 ` David L Stevens
2015-01-26 19:54 ` Sowmini Varadhan [this message]
2015-01-26 20:19   ` David L Stevens
2015-01-26 20:29     ` Sowmini Varadhan
2015-01-26 20:53       ` David L Stevens
2015-01-26 22:45         ` Sowmini Varadhan

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=20150126195403.GE6437@oracle.com \
    --to=sowmini.varadhan@oracle.com \
    --cc=davem@davemloft.net \
    --cc=david.stevens@oracle.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.