> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Wednesday, January 25, 2017 00:08
> To: Dexuan Cui <de...@microsoft.com>
> Cc: gre...@linuxfoundation.org; driverdev-de...@linuxdriverproject.org; KY
> Srinivasan <k...@microsoft.com>; Haiyang Zhang <haiya...@microsoft.com>;
> Stephen Hemminger <sthem...@microsoft.com>; o...@aepfle.de; Rolf
> Neugebauer <rolf.neugeba...@docker.com>; jasow...@redhat.com; linux-
> ker...@vger.kernel.org; a...@canonical.com
> 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 <de...@microsoft.com> 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 <sta...@vger.kernel.org> 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
