> -----Original Message----- > From: devel [mailto:driverdev-devel-bounces@xxxxxxxxxxxxxxxxxxxxxx] On > Behalf Of KY Srinivasan > Sent: Wednesday, March 11, 2015 8:32 PM > To: Jason Wang > Cc: olaf@xxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > gregkh@xxxxxxxxxxxxxxxxxxx; apw@xxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx > Subject: RE: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the > signalling logic with kick_q > > > > > -----Original Message----- > > From: Jason Wang [mailto:jasowang@xxxxxxxxxx] > > Sent: Wednesday, March 11, 2015 8:08 PM > > To: KY Srinivasan > > Cc: davem@xxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux- > > kernel@xxxxxxxxxxxxxxx; devel@xxxxxxxxxxxxxxxxxxxxxx; olaf@xxxxxxxxx; > > apw@xxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; KY Srinivasan > > Subject: Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in > > the signalling logic with kick_q > > > > > > > > On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@xxxxxxxxxxxxx> > > wrote: > > > When the caller specifies that signalling should be deferred, we > > > need to address the case where we are not able to place the current > > > packet because the buffer is full. In this case, we will signal the > > > host as some packets may have been placed on the ring buffer. > > > I would like to thank Jason Wang <jasowang@xxxxxxxxxx> for pointing > > > out this issue. > > > > > > Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx> > > > --- > > > drivers/hv/channel.c | 32 ++++++++++++++++++++++++++++++++ > > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index > > > e58cdb7..ae06ba9 100644 > > > --- a/drivers/hv/channel.c > > > +++ b/drivers/hv/channel.c > > > @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct > vmbus_channel > > > *channel, void *buffer, > > > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > > &signal); > > > > > > + /* > > > + * Here is the logic for signalling the host: > > > + * 1. If the host is already draining the ringbuffer, > > > + * don't signal. This is indicated by the parameter > > > + * "signal". > > > + * > > > + * 2. If we are not able to write, signal if kick_q is false. > > > + * kick_q being false indicates that we may have placed zero or > > > + * more packets with more packets to come. We will signal in > > > + * this case even if potentially we may have not placed any > > > + * packet. This is a rare enough condition that it should not > > > + * matter. > > > + */ > > > + > > > if ((ret == 0) && kick_q && signal) > > > vmbus_setevent(channel); > > > + else if ((ret != 0) && !kick_q) > > > + vmbus_setevent(channel); > > > > > > return ret; > > > } > > > @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct > > > vmbus_channel *channel, > > > > > > ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, > > > &signal); > > > > > > + /* > > > + * Here is the logic for signalling the host: > > > + * 1. If the host is already draining the ringbuffer, > > > + * don't signal. This is indicated by the parameter > > > + * "signal". > > > + * > > > + * 2. If we are not able to write, signal if kick_q is false. > > > + * kick_q being false indicates that we may have placed zero or > > > + * more packets with more packets to come. We will signal in > > > + * this case even if potentially we may have not placed any > > > + * packet. This is a rare enough condition that it should not > > > + * matter. > > > + */ > > > + > > > if ((ret == 0) && kick_q && signal) > > > vmbus_setevent(channel); > > > + else if ((ret != 0) && !kick_q) > > > + vmbus_setevent(channel); > > > > > > return ret; > > > } > > > -- > > > > Looks like we need to kick unconditionally here. Consider we may get - > > EAGAIN when we want to send the last skb (kick_q is true) from the > > list. We need kick host in this case. > > > > Btw, another method is let the driver to decide e.g exporting the > > vmbus_setevent() and call it in netvsc_start_xmit(). > > There is other state that governs if the host needs to be signaled and I don't > think we want to expose that to netvsc. In any case, netvsc will have to signal > the host when EGAIN is received. > We might as well do it in the vmbus driver. > > Thank you Jason, I will respin and resubmit the series. Looks like there may be a way to make the signaling more precise even in this case. I am going to experiment with it. K. Y > > K. Y > > > > > > > > _______________________________________________ > devel mailing list > devel@xxxxxxxxxxxxxxxxxxxxxx > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
