RE: [PATCH] Drivers: hv: vmbus: finally fix hv_need_to_signal_on_read()

[Dexuan Cui]

> From: Stephen Hemminger [mailto:stephen@xxxxxxxxxxxxxxxxxx]
> Sent: Wednesday, January 25, 2017 00:08
> To: Dexuan Cui <decui@xxxxxxxxxxxxx>
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; driverdev-devel@xxxxxxxxxxxxxxxxxxxxxx; KY
> Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>;
> Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>; olaf@xxxxxxxxx; Rolf
> Neugebauer <rolf.neugebauer@xxxxxxxxxx>; jasowang@xxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; apw@xxxxxxxxxxxxx
> Subject: Re: [PATCH] Drivers: hv: vmbus: finally fix hv_need_to_signal_on_read()
> 
> On Tue, 24 Jan 2017 06:54:46 +0000
> Dexuan Cui <decui@xxxxxxxxxxxxx> wrote:
> 
> > +static inline void
> > +init_cached_read_index(struct vmbus_channel *channel)
> > +{
> > +	struct hv_ring_buffer_info *rbi = &channel->inbound;
> > +
> > +	rbi->cached_read_index = rbi->ring_buffer->read_index;
> > +}
> 
> Looks good thanks. This should go in right away. Which versions are impacted?
> Should it also go to stable?

Yes, it needs to go to stable.
I have Cc-ed  <stable@xxxxxxxxxxxxxxx> in the patch's changelog, so it should be
included in the stable tree automatically.

As I checked against the kernels listed on the homapage of www.kernel.org, the
below versions are impacted:
v3.16.39
v3.18.47
v4.1.38
v4.8.17
v4.9.5
v4.10-rc5

It's interesting v4.4.44 is not impacted, but actually it needs both the 2 patches:
i.e. this patch, and the previous one:
Commit a389fcfd2cb5 ("Drivers: hv: vmbus: Fix signaling logic in hv_need_to_signal_on_read()")

> In a future patch, the API function names for interacting with the ring buffer
> should be changed to all have common prefix (hv_) and maybe do a little
> rethinking  about what needs to be in ring buffer and what could be local variables.
> 
> For example, the cached_read_index is only useful over the span of the loop
> reading from the ring buffer. For me, it would be cleaner with a ring_buffer
> iterator object which could abstract the API better.
> 
> 	struct vmbus_ringbuffer_iter iter;
> 
> 	vmbus_begin_read(&iter, channel);
> 	while ((desc = vmbus_next_read(&iter), channel) {
> 	    ...
> 	}
> 	vmbus_end_read(&iter, channel);

I agree. Please help to clean all these up.  :-)

Thanks,
-- Dexuan