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 |
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 >
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 >
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 >> >
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 --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