> From: Stephen Hemminger [mailto:[email protected]]
> Sent: Wednesday, January 25, 2017 00:08
> To: Dexuan Cui <[email protected]>
> Cc: [email protected]; [email protected]; KY
> Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>;
> Stephen Hemminger <[email protected]>; [email protected]; Rolf
> Neugebauer <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]
> 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 <[email protected]> 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  <[email protected]> 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
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to