On Sat, Sep 05, 2020 at 12:19:08AM +0000, Michael Kelley wrote: [...] > > > > @@ -462,7 +576,13 @@ static int __vmbus_open(struct vmbus_channel *newchannel, > > open_msg->openid = newchannel->offermsg.child_relid; > > open_msg->child_relid = newchannel->offermsg.child_relid; > > open_msg->ringbuffer_gpadlhandle = newchannel->ringbuffer_gpadlhandle; > > - open_msg->downstream_ringbuffer_pageoffset = newchannel- > > >ringbuffer_send_offset; > > + /* > > + * The unit of ->downstream_ringbuffer_pageoffset is HV_HYP_PAGE and > > + * the unit of ->ringbuffer_send_offset is PAGE, so here we first > > + * calculate it into bytes and then convert into HV_HYP_PAGE. > > + */ > > + open_msg->downstream_ringbuffer_pageoffset = > > + hv_ring_gpadl_send_offset(newchannel->ringbuffer_send_offset << PAGE_SHIFT) >> HV_HYP_PAGE_SHIFT; > > Line length? > Thanks for the review! I've resolved all your comments on wording for patch #2 and #4 in my local branch. For this line length issue, I fix it with two changes: 1) both the callsite of hv_ring_gpadl_send_offset() use ">> ..." to calculate the index in HV_HYP_PAGE, so I change the function to return offset in unit of HV_HYP_PAGE instead of bytes, and that can save us the ">> ..." here. 2) newchannel->ringbuffer_send_offset is read in the previous code of the function into local variable "send_pages", so I use it to replace the "newchannel->ringbuffer_send_offset" here. now the code is: open_msg->downstream_ringbuffer_pageoffset = hv_ring_gpadl_send_hvpgoffset(send_pages << PAGE_SHIFT); Regards, Boqun > > open_msg->target_vp = hv_cpu_number_to_vp_number(newchannel->target_cpu); > > > > if (userdatalen) > > @@ -556,7 +676,6 @@ int vmbus_open(struct vmbus_channel *newchannel, > > } > > EXPORT_SYMBOL_GPL(vmbus_open); > >
