diff mbox series

[SRU,Disco] iommu/iova: Separate atomic variables to improve performance

Message ID 20190614205748.19844-1-dann.frazier@canonical.com
State New
Headers show
Series [SRU,Disco] iommu/iova: Separate atomic variables to improve performance | expand

Commit Message

dann frazier June 14, 2019, 8:57 p.m. UTC 
From: Jinyu Qi <jinyuqi@huawei.com>

BugLink: https://bugs.launchpad.net/bugs/1832909

In struct iova_domain, there are three atomic variables, the former two
are about TLB flush counters which use atomic_add operation, anoter is
used to flush timer that use cmpxhg operation.
These variables are in the same cache line, so it will cause some
performance loss under the condition that many cores call queue_iova
function, Let's isolate the two type atomic variables to different
cache line to reduce cache line conflict.

Cc: Joerg Roedel <joro@8bytes.org>
Signed-off-by: Jinyu Qi <jinyuqi@huawei.com>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
(cherry picked from commit 14bd9a607f9082e7b5690c27e69072f2aeae0de4)
Signed-off-by: dann frazier <dann.frazier@canonical.com>
---
 include/linux/iova.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Stefan Bader June 28, 2019, 12:11 p.m. UTC | #1 
On 14.06.19 22:57, dann frazier wrote:
> From: Jinyu Qi <jinyuqi@huawei.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1832909
> 
> In struct iova_domain, there are three atomic variables, the former two
> are about TLB flush counters which use atomic_add operation, anoter is
> used to flush timer that use cmpxhg operation.
> These variables are in the same cache line, so it will cause some
> performance loss under the condition that many cores call queue_iova
> function, Let's isolate the two type atomic variables to different
> cache line to reduce cache line conflict.
> 
> Cc: Joerg Roedel <joro@8bytes.org>
> Signed-off-by: Jinyu Qi <jinyuqi@huawei.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> (cherry picked from commit 14bd9a607f9082e7b5690c27e69072f2aeae0de4)
> Signed-off-by: dann frazier <dann.frazier@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  include/linux/iova.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index 0b93bf96693ef..28a5128405f82 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h
> @@ -76,6 +76,14 @@ struct iova_domain {
>  	unsigned long	start_pfn;	/* Lower limit for this domain */
>  	unsigned long	dma_32bit_pfn;
>  	unsigned long	max32_alloc_size; /* Size of last failed allocation */
> +	struct iova_fq __percpu *fq;	/* Flush Queue */
> +
> +	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
> +						   have been started */
> +
> +	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
> +						   have been finished */
> +
>  	struct iova	anchor;		/* rbtree lookup anchor */
>  	struct iova_rcache rcaches[IOVA_RANGE_CACHE_MAX_SIZE];	/* IOVA range caches */
>  
> @@ -85,14 +93,6 @@ struct iova_domain {
>  	iova_entry_dtor entry_dtor;	/* IOMMU driver specific destructor for
>  					   iova entry */
>  
> -	struct iova_fq __percpu *fq;	/* Flush Queue */
> -
> -	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
> -						   have been started */
> -
> -	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
> -						   have been finished */
> -
>  	struct timer_list fq_timer;		/* Timer to regularily empty the
>  						   flush-queues */
>  	atomic_t fq_timer_on;			/* 1 when timer is active, 0
>
Connor Kuehl July 1, 2019, 6:45 p.m. UTC | #2 
On 6/14/19 1:57 PM, dann frazier wrote:
> From: Jinyu Qi <jinyuqi@huawei.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1832909
> 
> In struct iova_domain, there are three atomic variables, the former two
> are about TLB flush counters which use atomic_add operation, anoter is
> used to flush timer that use cmpxhg operation.
> These variables are in the same cache line, so it will cause some
> performance loss under the condition that many cores call queue_iova
> function, Let's isolate the two type atomic variables to different
> cache line to reduce cache line conflict.
> 
> Cc: Joerg Roedel <joro@8bytes.org>
> Signed-off-by: Jinyu Qi <jinyuqi@huawei.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> (cherry picked from commit 14bd9a607f9082e7b5690c27e69072f2aeae0de4)
> Signed-off-by: dann frazier <dann.frazier@canonical.com>

Acked-by: Connor Kuehl <connor.kuehl@canonical.com>

> ---
>  include/linux/iova.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index 0b93bf96693ef..28a5128405f82 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h
> @@ -76,6 +76,14 @@ struct iova_domain {
>  	unsigned long	start_pfn;	/* Lower limit for this domain */
>  	unsigned long	dma_32bit_pfn;
>  	unsigned long	max32_alloc_size; /* Size of last failed allocation */
> +	struct iova_fq __percpu *fq;	/* Flush Queue */
> +
> +	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
> +						   have been started */
> +
> +	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
> +						   have been finished */
> +
>  	struct iova	anchor;		/* rbtree lookup anchor */
>  	struct iova_rcache rcaches[IOVA_RANGE_CACHE_MAX_SIZE];	/* IOVA range caches */
>  
> @@ -85,14 +93,6 @@ struct iova_domain {
>  	iova_entry_dtor entry_dtor;	/* IOMMU driver specific destructor for
>  					   iova entry */
>  
> -	struct iova_fq __percpu *fq;	/* Flush Queue */
> -
> -	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
> -						   have been started */
> -
> -	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
> -						   have been finished */
> -
>  	struct timer_list fq_timer;		/* Timer to regularily empty the
>  						   flush-queues */
>  	atomic_t fq_timer_on;			/* 1 when timer is active, 0
>
Connor Kuehl July 1, 2019, 6:46 p.m. UTC | #3 
On 7/1/19 11:45 AM, Connor Kuehl wrote:
> On 6/14/19 1:57 PM, dann frazier wrote:
>> From: Jinyu Qi <jinyuqi@huawei.com>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1832909
>>
>> In struct iova_domain, there are three atomic variables, the former two
>> are about TLB flush counters which use atomic_add operation, anoter is
>> used to flush timer that use cmpxhg operation.
>> These variables are in the same cache line, so it will cause some
>> performance loss under the condition that many cores call queue_iova
>> function, Let's isolate the two type atomic variables to different
>> cache line to reduce cache line conflict.
>>
>> Cc: Joerg Roedel <joro@8bytes.org>
>> Signed-off-by: Jinyu Qi <jinyuqi@huawei.com>
>> Signed-off-by: Joerg Roedel <jroedel@suse.de>
>> (cherry picked from commit 14bd9a607f9082e7b5690c27e69072f2aeae0de4)
>> Signed-off-by: dann frazier <dann.frazier@canonical.com>
> 
> Acked-by: Connor Kuehl <connor.kuehl@canonical.com>

Sorry, I meant to put ACK in the e-mail subject.

Acked-by: Connor Kuehl <connor.kuehl@canonical.com>

> 
>> ---
>>  include/linux/iova.h | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/linux/iova.h b/include/linux/iova.h
>> index 0b93bf96693ef..28a5128405f82 100644
>> --- a/include/linux/iova.h
>> +++ b/include/linux/iova.h
>> @@ -76,6 +76,14 @@ struct iova_domain {
>>  	unsigned long	start_pfn;	/* Lower limit for this domain */
>>  	unsigned long	dma_32bit_pfn;
>>  	unsigned long	max32_alloc_size; /* Size of last failed allocation */
>> +	struct iova_fq __percpu *fq;	/* Flush Queue */
>> +
>> +	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
>> +						   have been started */
>> +
>> +	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
>> +						   have been finished */
>> +
>>  	struct iova	anchor;		/* rbtree lookup anchor */
>>  	struct iova_rcache rcaches[IOVA_RANGE_CACHE_MAX_SIZE];	/* IOVA range caches */
>>  
>> @@ -85,14 +93,6 @@ struct iova_domain {
>>  	iova_entry_dtor entry_dtor;	/* IOMMU driver specific destructor for
>>  					   iova entry */
>>  
>> -	struct iova_fq __percpu *fq;	/* Flush Queue */
>> -
>> -	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
>> -						   have been started */
>> -
>> -	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
>> -						   have been finished */
>> -
>>  	struct timer_list fq_timer;		/* Timer to regularily empty the
>>  						   flush-queues */
>>  	atomic_t fq_timer_on;			/* 1 when timer is active, 0
>>
>
Kleber Sacilotto de Souza July 2, 2019, 8:57 a.m. UTC | #4 
On 6/14/19 10:57 PM, dann frazier wrote:
> From: Jinyu Qi <jinyuqi@huawei.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1832909
> 
> In struct iova_domain, there are three atomic variables, the former two
> are about TLB flush counters which use atomic_add operation, anoter is
> used to flush timer that use cmpxhg operation.
> These variables are in the same cache line, so it will cause some
> performance loss under the condition that many cores call queue_iova
> function, Let's isolate the two type atomic variables to different
> cache line to reduce cache line conflict.
> 
> Cc: Joerg Roedel <joro@8bytes.org>
> Signed-off-by: Jinyu Qi <jinyuqi@huawei.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> (cherry picked from commit 14bd9a607f9082e7b5690c27e69072f2aeae0de4)
> Signed-off-by: dann frazier <dann.frazier@canonical.com>
> ---
>  include/linux/iova.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/iova.h b/include/linux/iova.h
> index 0b93bf96693ef..28a5128405f82 100644
> --- a/include/linux/iova.h
> +++ b/include/linux/iova.h
> @@ -76,6 +76,14 @@ struct iova_domain {
>  	unsigned long	start_pfn;	/* Lower limit for this domain */
>  	unsigned long	dma_32bit_pfn;
>  	unsigned long	max32_alloc_size; /* Size of last failed allocation */
> +	struct iova_fq __percpu *fq;	/* Flush Queue */
> +
> +	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
> +						   have been started */
> +
> +	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
> +						   have been finished */
> +
>  	struct iova	anchor;		/* rbtree lookup anchor */
>  	struct iova_rcache rcaches[IOVA_RANGE_CACHE_MAX_SIZE];	/* IOVA range caches */
>  
> @@ -85,14 +93,6 @@ struct iova_domain {
>  	iova_entry_dtor entry_dtor;	/* IOMMU driver specific destructor for
>  					   iova entry */
>  
> -	struct iova_fq __percpu *fq;	/* Flush Queue */
> -
> -	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
> -						   have been started */
> -
> -	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
> -						   have been finished */
> -
>  	struct timer_list fq_timer;		/* Timer to regularily empty the
>  						   flush-queues */
>  	atomic_t fq_timer_on;			/* 1 when timer is active, 0
> 

Applied to disco/master-next branch.

Thanks,
Kleber
diff mbox series

Patch

diff --git a/include/linux/iova.h b/include/linux/iova.h
index 0b93bf96693ef..28a5128405f82 100644
--- a/include/linux/iova.h
+++ b/include/linux/iova.h
@@ -76,6 +76,14 @@  struct iova_domain {
 	unsigned long	start_pfn;	/* Lower limit for this domain */
 	unsigned long	dma_32bit_pfn;
 	unsigned long	max32_alloc_size; /* Size of last failed allocation */
+	struct iova_fq __percpu *fq;	/* Flush Queue */
+
+	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
+						   have been started */
+
+	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
+						   have been finished */
+
 	struct iova	anchor;		/* rbtree lookup anchor */
 	struct iova_rcache rcaches[IOVA_RANGE_CACHE_MAX_SIZE];	/* IOVA range caches */
 
@@ -85,14 +93,6 @@  struct iova_domain {
 	iova_entry_dtor entry_dtor;	/* IOMMU driver specific destructor for
 					   iova entry */
 
-	struct iova_fq __percpu *fq;	/* Flush Queue */
-
-	atomic64_t	fq_flush_start_cnt;	/* Number of TLB flushes that
-						   have been started */
-
-	atomic64_t	fq_flush_finish_cnt;	/* Number of TLB flushes that
-						   have been finished */
-
 	struct timer_list fq_timer;		/* Timer to regularily empty the
 						   flush-queues */
 	atomic_t fq_timer_on;			/* 1 when timer is active, 0