Re: [PATCH v4 2/2] iommu/tegra-smmu: Expand mutex protection range
On Mon, Sep 28, 2020 at 11:13:25PM -0700, Nicolin Chen wrote: > This is used to protect potential race condition at use_count. > since probes of client drivers, calling attach_dev(), may run > concurrently. > > Signed-off-by: Nicolin Chen > --- > > Changelog > v3->v4: > * Fixed typo "Expend" => "Expand" > v2->v3: > * Renamed label "err_unlock" to "unlock" > v1->v2: > * N/A > > drivers/iommu/tegra-smmu.c | 34 +- > 1 file changed, 21 insertions(+), 13 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/2] iommu/tegra-smmu: Expand mutex protection range
29.09.2020 20:42, Dmitry Osipenko пишет: > 29.09.2020 09:13, Nicolin Chen пишет: >> This is used to protect potential race condition at use_count. >> since probes of client drivers, calling attach_dev(), may run >> concurrently. >> >> Signed-off-by: Nicolin Chen >> --- > > It's always better not to mix success and error code paths in order to > keep code readable, but not a big deal in the case of this particular > patch since the changed code is quite simple. Please try to avoid doing > this in the future patches. > > Also, please note that in general it's better to use locked/unlocked > versions for the functions like it's already done for > tegra_smmu_map/unmap, this will remove the need to maintain lockings in > the code. The code touched by this patch doesn't have complicated code > paths, so it's good enough to me. > > Reviewed-by: Dmitry Osipenko > Tested-by: Dmitry Osipenko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/2] iommu/tegra-smmu: Expand mutex protection range
On Tue, Sep 29, 2020 at 08:42:52PM +0300, Dmitry Osipenko wrote: > 29.09.2020 09:13, Nicolin Chen пишет: > > This is used to protect potential race condition at use_count. > > since probes of client drivers, calling attach_dev(), may run > > concurrently. > > > > Signed-off-by: Nicolin Chen > > --- > > It's always better not to mix success and error code paths in order to > keep code readable, but not a big deal in the case of this particular > patch since the changed code is quite simple. Please try to avoid doing > this in the future patches. > > Also, please note that in general it's better to use locked/unlocked > versions for the functions like it's already done for > tegra_smmu_map/unmap, this will remove the need to maintain lockings in > the code. The code touched by this patch doesn't have complicated code > paths, so it's good enough to me. > > Reviewed-by: Dmitry Osipenko Okay. Thanks for the review! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 2/2] iommu/tegra-smmu: Expand mutex protection range
29.09.2020 09:13, Nicolin Chen пишет: > This is used to protect potential race condition at use_count. > since probes of client drivers, calling attach_dev(), may run > concurrently. > > Signed-off-by: Nicolin Chen > --- It's always better not to mix success and error code paths in order to keep code readable, but not a big deal in the case of this particular patch since the changed code is quite simple. Please try to avoid doing this in the future patches. Also, please note that in general it's better to use locked/unlocked versions for the functions like it's already done for tegra_smmu_map/unmap, this will remove the need to maintain lockings in the code. The code touched by this patch doesn't have complicated code paths, so it's good enough to me. Reviewed-by: Dmitry Osipenko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 2/2] iommu/tegra-smmu: Expand mutex protection range
This is used to protect potential race condition at use_count.
since probes of client drivers, calling attach_dev(), may run
concurrently.
Signed-off-by: Nicolin Chen
---
Changelog
v3->v4:
* Fixed typo "Expend" => "Expand"
v2->v3:
* Renamed label "err_unlock" to "unlock"
v1->v2:
* N/A
drivers/iommu/tegra-smmu.c | 34 +-
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index ec4c9dafff95..6a3ecc334481 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -256,26 +256,19 @@ static int tegra_smmu_alloc_asid(struct tegra_smmu *smmu,
unsigned int *idp)
{
unsigned long id;
- mutex_lock(>lock);
-
id = find_first_zero_bit(smmu->asids, smmu->soc->num_asids);
- if (id >= smmu->soc->num_asids) {
- mutex_unlock(>lock);
+ if (id >= smmu->soc->num_asids)
return -ENOSPC;
- }
set_bit(id, smmu->asids);
*idp = id;
- mutex_unlock(>lock);
return 0;
}
static void tegra_smmu_free_asid(struct tegra_smmu *smmu, unsigned int id)
{
- mutex_lock(>lock);
clear_bit(id, smmu->asids);
- mutex_unlock(>lock);
}
static bool tegra_smmu_capable(enum iommu_cap cap)
@@ -420,17 +413,21 @@ static int tegra_smmu_as_prepare(struct tegra_smmu *smmu,
struct tegra_smmu_as *as)
{
u32 value;
- int err;
+ int err = 0;
+
+ mutex_lock(>lock);
if (as->use_count > 0) {
as->use_count++;
- return 0;
+ goto unlock;
}
as->pd_dma = dma_map_page(smmu->dev, as->pd, 0, SMMU_SIZE_PD,
DMA_TO_DEVICE);
- if (dma_mapping_error(smmu->dev, as->pd_dma))
- return -ENOMEM;
+ if (dma_mapping_error(smmu->dev, as->pd_dma)) {
+ err = -ENOMEM;
+ goto unlock;
+ }
/* We can't handle 64-bit DMA addresses */
if (!smmu_dma_addr_valid(smmu, as->pd_dma)) {
@@ -453,24 +450,35 @@ static int tegra_smmu_as_prepare(struct tegra_smmu *smmu,
as->smmu = smmu;
as->use_count++;
+ mutex_unlock(>lock);
+
return 0;
err_unmap:
dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD, DMA_TO_DEVICE);
+unlock:
+ mutex_unlock(>lock);
+
return err;
}
static void tegra_smmu_as_unprepare(struct tegra_smmu *smmu,
struct tegra_smmu_as *as)
{
- if (--as->use_count > 0)
+ mutex_lock(>lock);
+
+ if (--as->use_count > 0) {
+ mutex_unlock(>lock);
return;
+ }
tegra_smmu_free_asid(smmu, as->id);
dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD, DMA_TO_DEVICE);
as->smmu = NULL;
+
+ mutex_unlock(>lock);
}
static int tegra_smmu_attach_dev(struct iommu_domain *domain,
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
