[PATCH v3 0/2] improve the concurrency of arm_smmu_atc_inv_domain()

[PATCH v3 0/2] improve the concurrency of arm_smmu_atc_inv_domain()

[Zhen Lei]

v2 --> v3:
As Will Deacon's suggestion, I changed the lock type of
arm_smmu_domain.devices_lock from spinlock_t to rwlock_t, and I saw that the
performance is all right. And further use nr_ats_masters to quickly check have
no obvious effect, so I drop it.

Here is the performance data tested on my board:
Withou any change:
Jobs: 24 (f=24): [0.1% done] [9798M/0K /s] [2392K/0  iops] [09h:59m:13s]
Jobs: 24 (f=24): [0.1% done] [9782M/0K /s] [2388K/0  iops] [09h:59m:12s]
Jobs: 24 (f=24): [0.2% done] [9825M/0K /s] [2399K/0  iops] [09h:59m:11s]
Jobs: 24 (f=24): [0.2% done] [9836M/0K /s] [2401K/0  iops] [09h:59m:10s]

Change lock type from spinlock_t to rwlock_t:
Jobs: 24 (f=24): [0.1% done] [10996M/0K /s] [2685K/0  iops] [09h:59m:13s]
Jobs: 24 (f=24): [0.1% done] [10817M/0K /s] [2641K/0  iops] [09h:59m:12s]
Jobs: 24 (f=24): [0.2% done] [11083M/0K /s] [2706K/0  iops] [09h:59m:11s]
Jobs: 24 (f=24): [0.2% done] [10603M/0K /s] [2589K/0  iops] [09h:59m:10s]

Use nr_ats_masters:
Jobs: 24 (f=24): [0.2% done] [11105M/0K /s] [2711K/0  iops] [eta 09h:58m:40s]
Jobs: 24 (f=24): [0.2% done] [10511M/0K /s] [2566K/0  iops] [eta 09h:58m:39s]
Jobs: 24 (f=24): [0.2% done] [10560M/0K /s] [2578K/0  iops] [eta 09h:58m:38s]
Jobs: 24 (f=24): [0.2% done] [10494M/0K /s] [2562K/0  iops] [eta 09h:58m:37s]
Jobs: 24 (f=24): [0.2% done] [10528M/0K /s] [2570K/0  iops] [eta 09h:58m:36s]
Jobs: 24 (f=24): [0.3% done] [10638M/0K /s] [2597K/0  iops] [eta 09h:58m:35s]
Jobs: 24 (f=24): [0.3% done] [11158M/0K /s] [2724K/0  iops] [eta 09h:58m:34s]
Jobs: 24 (f=24): [0.3% done] [11386M/0K /s] [2780K/0  iops] [eta 09h:58m:32s]
Jobs: 24 (f=24): [0.3% done] [11118M/0K /s] [2714K/0  iops] [eta 09h:58m:32s]
Jobs: 24 (f=24): [0.3% done] [11031M/0K /s] [2693K/0  iops] [eta 09h:58m:31s]
Jobs: 24 (f=24): [0.3% done] [11361M/0K /s] [2774K/0  iops] [eta 09h:58m:30s]

v1 --> v2:
1. change the type of nr_ats_masters from atomic_t to int, and move its
   ind/dec operation from arm_smmu_enable_ats()/arm_smmu_disable_ats() to
   arm_smmu_attach_dev()/arm_smmu_detach_dev(), and protected by
   "spin_lock_irqsave(&smmu_domain->devices_lock, flags);"

Zhen Lei (2):
  iommu/arm-smmu-v3: don't add a master into smmu_domain before it's
    ready
  iommu/arm-smmu-v3: change the lock type of
    arm_smmu_domain.devices_lock

 drivers/iommu/arm-smmu-v3.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

-- 
1.8.3


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 1/2] iommu/arm-smmu-v3: don't add a master into smmu_domain before it's ready

[Zhen Lei]

Once a master has been added into smmu_domain->devices, it may immediately
be scaned in arm_smmu_unmap()-->arm_smmu_atc_inv_domain(). From a logical
point of view, the master should be added into smmu_domain after it has
completely initialized.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index c5c93e48b4dbdf7..e0dcc5d27291f8b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1958,10 +1958,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 
 	master->domain = smmu_domain;
 
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
-	list_add(&master->domain_head, &smmu_domain->devices);
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
-
 	if (smmu_domain->stage != ARM_SMMU_DOMAIN_BYPASS)
 		arm_smmu_enable_ats(master);
 
@@ -1969,6 +1965,10 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 		arm_smmu_write_ctx_desc(smmu, &smmu_domain->s1_cfg);
 
 	arm_smmu_install_ste_for_dev(master);
+
+	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	list_add(&master->domain_head, &smmu_domain->devices);
+	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 out_unlock:
 	mutex_unlock(&smmu_domain->init_mutex);
 	return ret;
-- 
1.8.3


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 2/2] iommu/arm-smmu-v3: change the lock type of arm_smmu_domain.devices_lock

[Zhen Lei]

A master add into or remove from smmu_domain->devices only happened in
arm_smmu_attach_dev()/arm_smmu_detach_dev(), the frequency of these
operations is very low. But we traverse smmu_domain->devices list in
arm_smmu_atc_inv_domain() are frequent. So change the protection from
spinlock to rwlock can improve concurrency, especially for the smmu
domain without ATS masters.

By the way, the cmdq has its own lock, so this change is safe.

Here is the performance data tested on my board:
Before:
Jobs: 24 (f=24): [0.1% done] [9798M/0K /s] [2392K/0  iops] [09h:59m:13s]
Jobs: 24 (f=24): [0.1% done] [9782M/0K /s] [2388K/0  iops] [09h:59m:12s]
Jobs: 24 (f=24): [0.2% done] [9825M/0K /s] [2399K/0  iops] [09h:59m:11s]
Jobs: 24 (f=24): [0.2% done] [9836M/0K /s] [2401K/0  iops] [09h:59m:10s]

After:
Jobs: 24 (f=24): [0.1% done] [10996M/0K /s] [2685K/0  iops] [09h:59m:13s]
Jobs: 24 (f=24): [0.1% done] [10817M/0K /s] [2641K/0  iops] [09h:59m:12s]
Jobs: 24 (f=24): [0.2% done] [11083M/0K /s] [2706K/0  iops] [09h:59m:11s]
Jobs: 24 (f=24): [0.2% done] [10603M/0K /s] [2589K/0  iops] [09h:59m:10s]

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Suggested-by: Will Deacon <will@kernel.org>
---
 drivers/iommu/arm-smmu-v3.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e0dcc5d27291f8b..eded2e7a5a0c444 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -641,7 +641,7 @@ struct arm_smmu_domain {
 	struct iommu_domain		domain;
 
 	struct list_head		devices;
-	spinlock_t			devices_lock;
+	rwlock_t			devices_lock;
 };
 
 struct arm_smmu_option_prop {
@@ -1536,10 +1536,10 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
 
 	arm_smmu_atc_inv_to_cmd(ssid, iova, size, &cmd);
 
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	read_lock_irqsave(&smmu_domain->devices_lock, flags);
 	list_for_each_entry(master, &smmu_domain->devices, domain_head)
 		ret |= arm_smmu_atc_inv_master(master, &cmd);
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+	read_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
 	return ret ? -ETIMEDOUT : 0;
 }
@@ -1648,7 +1648,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 
 	mutex_init(&smmu_domain->init_mutex);
 	INIT_LIST_HEAD(&smmu_domain->devices);
-	spin_lock_init(&smmu_domain->devices_lock);
+	rwlock_init(&smmu_domain->devices_lock);
 
 	return &smmu_domain->domain;
 }
@@ -1911,9 +1911,9 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
 	if (!smmu_domain)
 		return;
 
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	write_lock_irqsave(&smmu_domain->devices_lock, flags);
 	list_del(&master->domain_head);
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+	write_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
 	master->domain = NULL;
 	arm_smmu_install_ste_for_dev(master);
@@ -1966,9 +1966,9 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 
 	arm_smmu_install_ste_for_dev(master);
 
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	write_lock_irqsave(&smmu_domain->devices_lock, flags);
 	list_add(&master->domain_head, &smmu_domain->devices);
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+	write_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 out_unlock:
 	mutex_unlock(&smmu_domain->init_mutex);
 	return ret;
-- 
1.8.3


_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/2] improve the concurrency of arm_smmu_atc_inv_domain()

[Will Deacon]

On Fri, Aug 23, 2019 at 10:45:49AM +0800, Zhen Lei wrote:
> v2 --> v3:
> As Will Deacon's suggestion, I changed the lock type of
> arm_smmu_domain.devices_lock from spinlock_t to rwlock_t, and I saw that the
> performance is all right. And further use nr_ats_masters to quickly check have
> no obvious effect, so I drop it.

:/

I already sent two versions of a series fixing this without any locking at
all on the ->unmap() path, and you were on cc. I've also queued that stuff
up.

Did you not receive my patches?

v1: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038306.html
v2: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038374.html

Queued: https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/smmu-v3

Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/2] improve the concurrency of arm_smmu_atc_inv_domain()

[Leizhen (ThunderTown)]

On 2019/8/23 15:50, Will Deacon wrote:
> On Fri, Aug 23, 2019 at 10:45:49AM +0800, Zhen Lei wrote:
>> v2 --> v3:
>> As Will Deacon's suggestion, I changed the lock type of
>> arm_smmu_domain.devices_lock from spinlock_t to rwlock_t, and I saw that the
>> performance is all right. And further use nr_ats_masters to quickly check have
>> no obvious effect, so I drop it.
> 
> :/
> 
> I already sent two versions of a series fixing this without any locking at
> all on the ->unmap() path, and you were on cc. I've also queued that stuff
> up.
> 
> Did you not receive my patches?
Sorry, my message filter setting is a bit wrong, must contains
"linux-kernel@vger.kernel.org", I have corrected it.

> 
> v1: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038306.html
> v2: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038374.html
OK, I will test it when it's my turn to use the board.

> 
> Queued: https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/smmu-v3
> 
> Will
> 
> .
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/2] improve the concurrency of arm_smmu_atc_inv_domain()

[Will Deacon]

On Fri, Aug 23, 2019 at 04:06:52PM +0800, Leizhen (ThunderTown) wrote:
> 
> 
> On 2019/8/23 15:50, Will Deacon wrote:
> > On Fri, Aug 23, 2019 at 10:45:49AM +0800, Zhen Lei wrote:
> >> v2 --> v3:
> >> As Will Deacon's suggestion, I changed the lock type of
> >> arm_smmu_domain.devices_lock from spinlock_t to rwlock_t, and I saw that the
> >> performance is all right. And further use nr_ats_masters to quickly check have
> >> no obvious effect, so I drop it.
> > 
> > :/
> > 
> > I already sent two versions of a series fixing this without any locking at
> > all on the ->unmap() path, and you were on cc. I've also queued that stuff
> > up.
> > 
> > Did you not receive my patches?
> Sorry, my message filter setting is a bit wrong, must contains
> "linux-kernel@vger.kernel.org", I have corrected it.

Ha, sounds like the opposite of my email filter ;)

> > v1: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038306.html
> > v2: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038374.html
> OK, I will test it when it's my turn to use the board.

Thanks, although I plan to send it to Joerg today so any changes will need
to go on top. Does your testing involve ATS, or just non-ATS devices? I've
tested the latter locally, although I haven't looked at the performance
since most of the patches are trying to fix the enable/disable ordering.

Thanks,

Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/2] improve the concurrency of arm_smmu_atc_inv_domain()

[Leizhen (ThunderTown)]

On 2019/8/23 16:37, Will Deacon wrote:
> On Fri, Aug 23, 2019 at 04:06:52PM +0800, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2019/8/23 15:50, Will Deacon wrote:
>>> On Fri, Aug 23, 2019 at 10:45:49AM +0800, Zhen Lei wrote:
>>>> v2 --> v3:
>>>> As Will Deacon's suggestion, I changed the lock type of
>>>> arm_smmu_domain.devices_lock from spinlock_t to rwlock_t, and I saw that the
>>>> performance is all right. And further use nr_ats_masters to quickly check have
>>>> no obvious effect, so I drop it.
>>>
>>> :/
>>>
>>> I already sent two versions of a series fixing this without any locking at
>>> all on the ->unmap() path, and you were on cc. I've also queued that stuff
>>> up.
>>>
>>> Did you not receive my patches?
>> Sorry, my message filter setting is a bit wrong, must contains
>> "linux-kernel@vger.kernel.org", I have corrected it.
> 
> Ha, sounds like the opposite of my email filter ;)
> 
>>> v1: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038306.html
>>> v2: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038374.html
>> OK, I will test it when it's my turn to use the board.
> 
> Thanks, although I plan to send it to Joerg today so any changes will need
> to go on top. Does your testing involve ATS, or just non-ATS devices? I've
I also currently only have non-ATS devices. 

> tested the latter locally, although I haven't looked at the performance
> since most of the patches are trying to fix the enable/disable ordering.
> 
> Thanks,
> 
> Will
> 
> .
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/2] improve the concurrency of arm_smmu_atc_inv_domain()

[Leizhen (ThunderTown)]

On 2019/8/23 16:06, Leizhen (ThunderTown) wrote:
> 
> 
> On 2019/8/23 15:50, Will Deacon wrote:
>> On Fri, Aug 23, 2019 at 10:45:49AM +0800, Zhen Lei wrote:
>>> v2 --> v3:
>>> As Will Deacon's suggestion, I changed the lock type of
>>> arm_smmu_domain.devices_lock from spinlock_t to rwlock_t, and I saw that the
>>> performance is all right. And further use nr_ats_masters to quickly check have
>>> no obvious effect, so I drop it.
>>
>> :/
>>
>> I already sent two versions of a series fixing this without any locking at
>> all on the ->unmap() path, and you were on cc. I've also queued that stuff
>> up.
>>
>> Did you not receive my patches?
> Sorry, my message filter setting is a bit wrong, must contains
> "linux-kernel@vger.kernel.org", I have corrected it.
> 
>>
>> v1: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038306.html
>> v2: https://lists.linuxfoundation.org/pipermail/iommu/2019-August/038374.html
> OK, I will test it when it's my turn to use the board.

The test result shows good to me, without these patches, it's about 22xx-23xx

Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11160M/0K /s] [2725K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11165M/0K /s] [2726K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11220M/0K /s] [2739K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11189M/0K /s] [2732K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11082M/0K /s] [2705K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11003M/0K /s] [2686K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11412M/0K /s] [2786K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11415M/0K /s] [2787K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11214M/0K /s] [2738K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [11054M/0K /s] [2699K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [10733M/0K /s] [2620K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.6% done] [10772M/0K /s] [2630K/0 
Jobs: 24 (f=24): [RRRRRRRRRRRRRRRRRRRRRRRR] [0.7% done] [10772M/0K /s] [2630K/0 

> 
>>
>> Queued: https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/smmu-v3
>>
>> Will
>>
>> .
>>
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu