* [PATCH] iommu: Fix a boundary issue to avoid performance drop
@ 2021-03-25 3:38 chenxiang
2021-03-25 9:43 ` Will Deacon
0 siblings, 1 reply; 5+ messages in thread
From: chenxiang @ 2021-03-25 3:38 UTC (permalink / raw)
To: yong.wu, robin.murphy, will; +Cc: iommu, linuxarm
From: Xiang Chen <chenxiang66@hisilicon.com>
After the change of patch ("iommu: Switch gather->end to the
inclusive end"), the performace drops from 1600+K IOPS to 1200K in our
kunpeng ARM64 platform.
We find that the range [start1, end1) actually is joint from the range
[end1, end2), but it is considered as disjoint after the change,
so it needs more times of TLB sync, and spends more time on it.
So fix the boundary issue to avoid performance drop.
Fixes: 862c3715de8f ("iommu: Switch gather->end to the inclusive end")
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
---
include/linux/iommu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index ae8eddd..4d5bcc2 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -547,7 +547,7 @@ static inline void iommu_iotlb_gather_add_page(struct iommu_domain *domain,
* structure can be rewritten.
*/
if (gather->pgsize != size ||
- end < gather->start || start > gather->end) {
+ end + 1 < gather->start || start > gather->end + 1) {
if (gather->pgsize)
iommu_iotlb_sync(domain, gather);
gather->pgsize = size;
--
2.8.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu: Fix a boundary issue to avoid performance drop
2021-03-25 3:38 [PATCH] iommu: Fix a boundary issue to avoid performance drop chenxiang
@ 2021-03-25 9:43 ` Will Deacon
2021-03-25 10:48 ` Robin Murphy
2021-04-07 8:24 ` Joerg Roedel
0 siblings, 2 replies; 5+ messages in thread
From: Will Deacon @ 2021-03-25 9:43 UTC (permalink / raw)
To: chenxiang, joro; +Cc: iommu, robin.murphy, linuxarm
[+ Joerg]
On Thu, Mar 25, 2021 at 11:38:24AM +0800, chenxiang wrote:
> From: Xiang Chen <chenxiang66@hisilicon.com>
>
> After the change of patch ("iommu: Switch gather->end to the
> inclusive end"), the performace drops from 1600+K IOPS to 1200K in our
> kunpeng ARM64 platform.
> We find that the range [start1, end1) actually is joint from the range
> [end1, end2), but it is considered as disjoint after the change,
> so it needs more times of TLB sync, and spends more time on it.
> So fix the boundary issue to avoid performance drop.
>
> Fixes: 862c3715de8f ("iommu: Switch gather->end to the inclusive end")
> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> ---
> include/linux/iommu.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index ae8eddd..4d5bcc2 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -547,7 +547,7 @@ static inline void iommu_iotlb_gather_add_page(struct iommu_domain *domain,
> * structure can be rewritten.
> */
> if (gather->pgsize != size ||
> - end < gather->start || start > gather->end) {
> + end + 1 < gather->start || start > gather->end + 1) {
> if (gather->pgsize)
> iommu_iotlb_sync(domain, gather);
> gather->pgsize = size;
Urgh, I must say I much preferred these things being exclusive, but this
looks like a necessary fix:
Acked-by: Will Deacon <will@kernel.org>
I wonder whether we should've just made these things u64s instead...
Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu: Fix a boundary issue to avoid performance drop
2021-03-25 9:43 ` Will Deacon
@ 2021-03-25 10:48 ` Robin Murphy
2021-03-25 11:08 ` Will Deacon
2021-04-07 8:24 ` Joerg Roedel
1 sibling, 1 reply; 5+ messages in thread
From: Robin Murphy @ 2021-03-25 10:48 UTC (permalink / raw)
To: Will Deacon, chenxiang, joro; +Cc: iommu, linuxarm
On 2021-03-25 09:43, Will Deacon wrote:
> [+ Joerg]
>
> On Thu, Mar 25, 2021 at 11:38:24AM +0800, chenxiang wrote:
>> From: Xiang Chen <chenxiang66@hisilicon.com>
>>
>> After the change of patch ("iommu: Switch gather->end to the
>> inclusive end"), the performace drops from 1600+K IOPS to 1200K in our
>> kunpeng ARM64 platform.
>> We find that the range [start1, end1) actually is joint from the range
>> [end1, end2), but it is considered as disjoint after the change,
>> so it needs more times of TLB sync, and spends more time on it.
>> So fix the boundary issue to avoid performance drop.
>>
>> Fixes: 862c3715de8f ("iommu: Switch gather->end to the inclusive end")
>> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
>> ---
>> include/linux/iommu.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index ae8eddd..4d5bcc2 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -547,7 +547,7 @@ static inline void iommu_iotlb_gather_add_page(struct iommu_domain *domain,
>> * structure can be rewritten.
>> */
>> if (gather->pgsize != size ||
>> - end < gather->start || start > gather->end) {
>> + end + 1 < gather->start || start > gather->end + 1) {
>> if (gather->pgsize)
>> iommu_iotlb_sync(domain, gather);
>> gather->pgsize = size;
>
> Urgh, I must say I much preferred these things being exclusive, but this
> looks like a necessary fix:
>
> Acked-by: Will Deacon <will@kernel.org>
>
> I wonder whether we should've just made these things u64s instead...
They'd have to be u65 or larger to represent a range ending on the
highest valid TTBR1 address, which is a thing we support ;)
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu: Fix a boundary issue to avoid performance drop
2021-03-25 10:48 ` Robin Murphy
@ 2021-03-25 11:08 ` Will Deacon
0 siblings, 0 replies; 5+ messages in thread
From: Will Deacon @ 2021-03-25 11:08 UTC (permalink / raw)
To: Robin Murphy; +Cc: linuxarm, iommu
On Thu, Mar 25, 2021 at 10:48:17AM +0000, Robin Murphy wrote:
> On 2021-03-25 09:43, Will Deacon wrote:
> > [+ Joerg]
> >
> > On Thu, Mar 25, 2021 at 11:38:24AM +0800, chenxiang wrote:
> > > From: Xiang Chen <chenxiang66@hisilicon.com>
> > >
> > > After the change of patch ("iommu: Switch gather->end to the
> > > inclusive end"), the performace drops from 1600+K IOPS to 1200K in our
> > > kunpeng ARM64 platform.
> > > We find that the range [start1, end1) actually is joint from the range
> > > [end1, end2), but it is considered as disjoint after the change,
> > > so it needs more times of TLB sync, and spends more time on it.
> > > So fix the boundary issue to avoid performance drop.
> > >
> > > Fixes: 862c3715de8f ("iommu: Switch gather->end to the inclusive end")
> > > Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> > > ---
> > > include/linux/iommu.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > > index ae8eddd..4d5bcc2 100644
> > > --- a/include/linux/iommu.h
> > > +++ b/include/linux/iommu.h
> > > @@ -547,7 +547,7 @@ static inline void iommu_iotlb_gather_add_page(struct iommu_domain *domain,
> > > * structure can be rewritten.
> > > */
> > > if (gather->pgsize != size ||
> > > - end < gather->start || start > gather->end) {
> > > + end + 1 < gather->start || start > gather->end + 1) {
> > > if (gather->pgsize)
> > > iommu_iotlb_sync(domain, gather);
> > > gather->pgsize = size;
> >
> > Urgh, I must say I much preferred these things being exclusive, but this
> > looks like a necessary fix:
> >
> > Acked-by: Will Deacon <will@kernel.org>
> >
> > I wonder whether we should've just made these things u64s instead...
>
> They'd have to be u65 or larger to represent a range ending on the highest
> valid TTBR1 address, which is a thing we support ;)
Damn, yes, I forgot about TTBR1. This reminds me of the trick with the
carry flag when checking user addresses.
Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iommu: Fix a boundary issue to avoid performance drop
2021-03-25 9:43 ` Will Deacon
2021-03-25 10:48 ` Robin Murphy
@ 2021-04-07 8:24 ` Joerg Roedel
1 sibling, 0 replies; 5+ messages in thread
From: Joerg Roedel @ 2021-04-07 8:24 UTC (permalink / raw)
To: Will Deacon; +Cc: iommu, robin.murphy, linuxarm
On Thu, Mar 25, 2021 at 09:43:45AM +0000, Will Deacon wrote:
> Urgh, I must say I much preferred these things being exclusive, but this
> looks like a necessary fix:
>
> Acked-by: Will Deacon <will@kernel.org>
Applied for v5.12.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-07 8:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 3:38 [PATCH] iommu: Fix a boundary issue to avoid performance drop chenxiang
2021-03-25 9:43 ` Will Deacon
2021-03-25 10:48 ` Robin Murphy
2021-03-25 11:08 ` Will Deacon
2021-04-07 8:24 ` Joerg Roedel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).