Re: [PATCH net-next] sunvnet: Fixes to avoid soft lockup in sunvnet

[Sowmini Varadhan <sowmini.varadhan@oracle.com>]

On (08/11/14 13:56), David Miller wrote:
> 
> Please don't fix multiple things in one huge patch, split up the
> changes into separate easily reviewed patches.

So there were 4 different sources of soft-lockup here, each of them
was just a few lines of change. Do you really want these as 4 patches?

I have no issues doing that (and also fixing the rest below)
but I just wanted to make sure that's really what's being asked for- 
since it seems like a lot of small patches, for something that made 
sense for me to test as one piece.


> 
> > +/*
> > + * Heuristic for the number of times to exponentially backoff and
> > + * retry sending an LDC trigger when EAGAIN is encountered
> > + */
> 
> In the networking, comments are formatted:
> 
> /* Like
>  * this.
>  */
> 
> > +			printk(KERN_INFO 
> > +			    "ECONNRESET %x:%x:%x:%x:%x:%x\n",
> > +			    port->raddr[0], port->raddr[1],
> > +			    port->raddr[2], port->raddr[3],
> > +			    port->raddr[4], port->raddr[5]);
> 
> This is not indented correctly.  The second and subsequent lines
> of the printk() function call should start at exactly the first
> column after the openning parenthesis of the function call.
> 
> > +	/*
> > +	 * Kick off a tasklet to wake the queue.  We cannot call 
> > +	 * maybe_tx_wakeup directly here because we could deadlock on
> > +	 * netif_tx_lock() with dev_watchdog()
> > +	 */
> 
> Comment formatting.
> 
> > +	/*
> > +	 * We don't rely on the ACKs to free the skb in vnet_start_xmit(),
> > +	 * thus it is safe to not set VIO_ACK_ENABLE for each transmission:
> > +	 * the protocol itself does not require it as long as the peer
> > +	 * sends a VIO_SUBTYPE_ACK for VIO_DRING_STOPPED. 
> > +	 *
> > +	 * An ACK for every packet in the ring is expensive as the
> > +	 * sending of LDC messages is slow and affects performance.
> > +	 */
> 
> Likewise.