> -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > Sent: Thursday, January 05, 2017 3:40 AM > To: Long Li <longli@xxxxxxxxxxxxx> > Cc: KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang > <haiyangz@xxxxxxxxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] hv: use substraction to update ring buffer index > > On Wed, Jan 04, 2017 at 08:08:22PM -0800, Long Li wrote: > > From: Long Li <longli@xxxxxxxxxxxxx> > > > > The ring buffer code uses %= to calculate index. For x86/64, %= > > compiles to div, more than 10 times slower than sub. > > > > Replace div with sub for this data heavy code path. > > > > Signed-off-by: Long Li <longli@xxxxxxxxxxxxx> > > --- > > drivers/hv/ring_buffer.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index > > cd49cb1..f8eee6e 100644 > > --- a/drivers/hv/ring_buffer.c > > +++ b/drivers/hv/ring_buffer.c > > @@ -135,7 +135,8 @@ hv_get_next_readlocation_withoffset(struct > hv_ring_buffer_info *ring_info, > > u32 next = ring_info->ring_buffer->read_index; > > > > next += offset; > > - next %= ring_info->ring_datasize; > > + if (next >= ring_info->ring_datasize) > > + next -= ring_info->ring_datasize; > > I take it that we trust that offset is roughly correct and not more than 2x > ring_info->ring_datasize? I guess there is only one caller so it's probably > true... Yes, you are right. It's not possible that we are getting to 2x ring_datasize, because it's not possible to transfer data more than ring_datasize over ring buffer. > > > > > return next; > > } > > @@ -179,7 +180,8 @@ static u32 hv_copyfrom_ringbuffer( > > memcpy(dest, ring_buffer + start_read_offset, destlen); > > > > start_read_offset += destlen; > > - start_read_offset %= ring_buffer_size; > > + if (start_read_offset >= ring_buffer_size) > > + start_read_offset -= ring_buffer_size; > > I totally don't understand the original code here. We do the memset and > then we verify that we are not copying beyond the end of the ring buffer? If > feels like we should verify that offset + destlen aren't more than > ring_buffer_size before we do the memcpy(). The ring buffer pages are mapped to wraparound 2x virtual address space. Please see hv_ringbuffer_init(). The call to vmap() setup this virtual address space. So we can use memcpy across the last page. > > regards, > dan carpenter > Thanks for reviewing! Long _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
