[v3,13/15] vhost: cache address translation result

Message ID 20190925171329.63734-14-yong.liu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vhost packed ring performance optimization |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Marvin Liu Sept. 25, 2019, 5:13 p.m. UTC
  Cache address translation result and use it in next translation. Due
to limited regions are supported, buffers are most likely in same
region when doing data transmission.

Signed-off-by: Marvin Liu <yong.liu@intel.com>
  

Comments

Tiwei Bie Sept. 26, 2019, 5:32 a.m. UTC | #1
On Thu, Sep 26, 2019 at 01:13:27AM +0800, Marvin Liu wrote:
> Cache address translation result and use it in next translation. Due
> to limited regions are supported, buffers are most likely in same
> region when doing data transmission.
> 
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
> 
> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
> index 7fb172912..d90235cd6 100644
> --- a/lib/librte_vhost/rte_vhost.h
> +++ b/lib/librte_vhost/rte_vhost.h
> @@ -91,10 +91,18 @@ struct rte_vhost_mem_region {
>  	int fd;
>  };
>  
> +struct rte_vhost_mem_region_cache {
> +	uint64_t guest_phys_addr;
> +	uint64_t guest_phys_addr_end;
> +	int64_t host_user_addr_offset;
> +	uint64_t size;
> +};
> +
>  /**
>   * Memory structure includes region and mapping information.
>   */
>  struct rte_vhost_memory {
> +	struct rte_vhost_mem_region_cache cache_region;

This breaks ABI.

>  	uint32_t nregions;
>  	struct rte_vhost_mem_region regions[];
>  };
> @@ -232,11 +240,30 @@ rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
>  	struct rte_vhost_mem_region *r;
>  	uint32_t i;
>  
> +	struct rte_vhost_mem_region_cache *r_cache;
> +	/* check with cached region */
> +	r_cache = &mem->cache_region;
> +	if (likely(gpa >= r_cache->guest_phys_addr && gpa <
> +		   r_cache->guest_phys_addr_end)) {
> +		if (unlikely(*len > r_cache->guest_phys_addr_end - gpa))
> +			*len = r_cache->guest_phys_addr_end - gpa;
> +
> +		return gpa - r_cache->host_user_addr_offset;
> +	}

Does this help a lot in performance?
We can implement this caching for builtin backend first.


> +
> +
>  	for (i = 0; i < mem->nregions; i++) {
>  		r = &mem->regions[i];
>  		if (gpa >= r->guest_phys_addr &&
>  		    gpa <  r->guest_phys_addr + r->size) {
>  
> +			r_cache->guest_phys_addr = r->guest_phys_addr;
> +			r_cache->guest_phys_addr_end = r->guest_phys_addr +
> +						       r->size;
> +			r_cache->size = r->size;
> +			r_cache->host_user_addr_offset = r->guest_phys_addr -
> +							 r->host_user_addr;
> +
>  			if (unlikely(*len > r->guest_phys_addr + r->size - gpa))
>  				*len = r->guest_phys_addr + r->size - gpa;
>  
> -- 
> 2.17.1
>
  
Marvin Liu Oct. 9, 2019, 2:08 a.m. UTC | #2
> -----Original Message-----
> From: Bie, Tiwei
> Sent: Thursday, September 26, 2019 1:32 PM
> To: Liu, Yong <yong.liu@intel.com>
> Cc: maxime.coquelin@redhat.com; Wang, Zhihong <zhihong.wang@intel.com>;
> stephen@networkplumber.org; gavin.hu@arm.com; dev@dpdk.org
> Subject: Re: [PATCH v3 13/15] vhost: cache address translation result
> 
> On Thu, Sep 26, 2019 at 01:13:27AM +0800, Marvin Liu wrote:
> > Cache address translation result and use it in next translation. Due
> > to limited regions are supported, buffers are most likely in same
> > region when doing data transmission.
> >
> > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> >
> > diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
> > index 7fb172912..d90235cd6 100644
> > --- a/lib/librte_vhost/rte_vhost.h
> > +++ b/lib/librte_vhost/rte_vhost.h
> > @@ -91,10 +91,18 @@ struct rte_vhost_mem_region {
> >  	int fd;
> >  };
> >
> > +struct rte_vhost_mem_region_cache {
> > +	uint64_t guest_phys_addr;
> > +	uint64_t guest_phys_addr_end;
> > +	int64_t host_user_addr_offset;
> > +	uint64_t size;
> > +};
> > +
> >  /**
> >   * Memory structure includes region and mapping information.
> >   */
> >  struct rte_vhost_memory {
> > +	struct rte_vhost_mem_region_cache cache_region;
> 
> This breaks ABI.
> 
Got, will remove it as no clear performance gain with this patch.

> >  	uint32_t nregions;
> >  	struct rte_vhost_mem_region regions[];
> >  };
> > @@ -232,11 +240,30 @@ rte_vhost_va_from_guest_pa(struct rte_vhost_memory
> *mem,
> >  	struct rte_vhost_mem_region *r;
> >  	uint32_t i;
> >
> > +	struct rte_vhost_mem_region_cache *r_cache;
> > +	/* check with cached region */
> > +	r_cache = &mem->cache_region;
> > +	if (likely(gpa >= r_cache->guest_phys_addr && gpa <
> > +		   r_cache->guest_phys_addr_end)) {
> > +		if (unlikely(*len > r_cache->guest_phys_addr_end - gpa))
> > +			*len = r_cache->guest_phys_addr_end - gpa;
> > +
> > +		return gpa - r_cache->host_user_addr_offset;
> > +	}
> 
> Does this help a lot in performance?
> We can implement this caching for builtin backend first.
> 
Tiwei,

It won’t help too much in performance as region number will be 1 at most of times.
Will remove cache function in next version.

Thanks,
Marvin
> 
> > +
> > +
> >  	for (i = 0; i < mem->nregions; i++) {
> >  		r = &mem->regions[i];
> >  		if (gpa >= r->guest_phys_addr &&
> >  		    gpa <  r->guest_phys_addr + r->size) {
> >
> > +			r_cache->guest_phys_addr = r->guest_phys_addr;
> > +			r_cache->guest_phys_addr_end = r->guest_phys_addr +
> > +						       r->size;
> > +			r_cache->size = r->size;
> > +			r_cache->host_user_addr_offset = r->guest_phys_addr -
> > +							 r->host_user_addr;
> > +
> >  			if (unlikely(*len > r->guest_phys_addr + r->size - gpa))
> >  				*len = r->guest_phys_addr + r->size - gpa;
> >
> > --
> > 2.17.1
> >
  

Patch

diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index 7fb172912..d90235cd6 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -91,10 +91,18 @@  struct rte_vhost_mem_region {
 	int fd;
 };
 
+struct rte_vhost_mem_region_cache {
+	uint64_t guest_phys_addr;
+	uint64_t guest_phys_addr_end;
+	int64_t host_user_addr_offset;
+	uint64_t size;
+};
+
 /**
  * Memory structure includes region and mapping information.
  */
 struct rte_vhost_memory {
+	struct rte_vhost_mem_region_cache cache_region;
 	uint32_t nregions;
 	struct rte_vhost_mem_region regions[];
 };
@@ -232,11 +240,30 @@  rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
 	struct rte_vhost_mem_region *r;
 	uint32_t i;
 
+	struct rte_vhost_mem_region_cache *r_cache;
+	/* check with cached region */
+	r_cache = &mem->cache_region;
+	if (likely(gpa >= r_cache->guest_phys_addr && gpa <
+		   r_cache->guest_phys_addr_end)) {
+		if (unlikely(*len > r_cache->guest_phys_addr_end - gpa))
+			*len = r_cache->guest_phys_addr_end - gpa;
+
+		return gpa - r_cache->host_user_addr_offset;
+	}
+
+
 	for (i = 0; i < mem->nregions; i++) {
 		r = &mem->regions[i];
 		if (gpa >= r->guest_phys_addr &&
 		    gpa <  r->guest_phys_addr + r->size) {
 
+			r_cache->guest_phys_addr = r->guest_phys_addr;
+			r_cache->guest_phys_addr_end = r->guest_phys_addr +
+						       r->size;
+			r_cache->size = r->size;
+			r_cache->host_user_addr_offset = r->guest_phys_addr -
+							 r->host_user_addr;
+
 			if (unlikely(*len > r->guest_phys_addr + r->size - gpa))
 				*len = r->guest_phys_addr + r->size - gpa;