Messages in this thread | | | Subject | Re: [PATCH RFC 2/8] iommu/arm-smmu-v3: Add and use static helper function arm_smmu_cmdq_issue_cmd_with_sync() | From | "Leizhen (ThunderTown)" <> | Date | Wed, 11 Aug 2021 10:16:39 +0800 |
| |
On 2021/8/11 2:24, Will Deacon wrote: > On Sat, Jun 26, 2021 at 07:01:24PM +0800, Zhen Lei wrote: >> The obvious key to the performance optimization of commit 587e6c10a7ce >> ("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is >> to allow multiple cores to insert commands in parallel after a brief mutex >> contention. >> >> Obviously, inserting as many commands at a time as possible can reduce the >> number of times the mutex contention participates, thereby improving the >> overall performance. At least it reduces the number of calls to function >> arm_smmu_cmdq_issue_cmdlist(). >> >> Therefore, function arm_smmu_cmdq_issue_cmd_with_sync() is added to insert >> the 'cmd+sync' commands at a time. >> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >> --- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33 +++++++++++++-------- >> 1 file changed, 21 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> index 2433d3c29b49ff2..a5361153ca1d6a4 100644 >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> @@ -858,11 +858,25 @@ static int arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu, >> return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, false); >> } >> >> -static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu) >> +static int __maybe_unused arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu) >> { >> return arm_smmu_cmdq_issue_cmdlist(smmu, NULL, 0, true); >> } >> >> +static int arm_smmu_cmdq_issue_cmd_with_sync(struct arm_smmu_device *smmu, >> + struct arm_smmu_cmdq_ent *ent) >> +{ >> + u64 cmd[CMDQ_ENT_DWORDS]; >> + >> + if (arm_smmu_cmdq_build_cmd(cmd, ent)) { >> + dev_warn(smmu->dev, "ignoring unknown CMDQ opcode 0x%x\n", >> + ent->opcode); >> + return -EINVAL; >> + } >> + >> + return arm_smmu_cmdq_issue_cmdlist(smmu, cmd, 1, true); >> +} > > This function is almost identical to arm_smmu_cmdq_issue_cmd(). How about > moving the guts out into a helper: > > static int __arm_smmu_cmdq_issue_cmd(struct arm_smmu_device *smmu, > struct arm_smmu_cmdq_ent *ent, > bool sync); > > and then having arm_smmu_cmdq_issue_cmd_with_sync() and > arm_smmu_cmdq_issue_cmd() wrap that?
OK, I will do it.
How about remove arm_smmu_cmdq_issue_sync()? It's unused now.
> > Will > . >
| |