* [PATCH v2 1/2] iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind
2023-06-09 10:51 [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues Vasant Hegde
@ 2023-06-09 10:51 ` Vasant Hegde
2023-06-09 10:51 ` [PATCH v2 2/2] iommu/amd/iommu_v2: Clear pasid state in free path Vasant Hegde
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Vasant Hegde @ 2023-06-09 10:51 UTC (permalink / raw)
To: iommu, joro; +Cc: suravee.suthikulpanit, jgg, dmarcovitch, Vasant Hegde
From: Daniel Marcovitch <dmarcovitch@nvidia.com>
When unbinding pasid - a race condition exists vs outstanding page faults.
To prevent this, the pasid_state object contains a refcount.
* set to 1 on pasid bind
* incremented on each ppr notification start
* decremented on each ppr notification done
* decremented on pasid unbind
Since refcount_dec assumes that refcount will never reach 0:
the current implementation causes the following to be invoked on
pasid unbind:
REFCOUNT_WARN("decrement hit 0; leaking memory")
Fix this issue by changing refcount_dec to refcount_dec_and_test
to explicitly handle refcount=1.
Fixes: 8bc54824da4e ("iommu/amd: Convert from atomic_t to refcount_t on pasid_state->count")
Signed-off-by: Daniel Marcovitch <dmarcovitch@nvidia.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
drivers/iommu/amd/iommu_v2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
index 864e4ffb6aa9..858748baae0b 100644
--- a/drivers/iommu/amd/iommu_v2.c
+++ b/drivers/iommu/amd/iommu_v2.c
@@ -262,8 +262,8 @@ static void put_pasid_state(struct pasid_state *pasid_state)
static void put_pasid_state_wait(struct pasid_state *pasid_state)
{
- refcount_dec(&pasid_state->count);
- wait_event(pasid_state->wq, !refcount_read(&pasid_state->count));
+ if (!refcount_dec_and_test(&pasid_state->count))
+ wait_event(pasid_state->wq, !refcount_read(&pasid_state->count));
free_pasid_state(pasid_state);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] iommu/amd/iommu_v2: Clear pasid state in free path
2023-06-09 10:51 [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues Vasant Hegde
2023-06-09 10:51 ` [PATCH v2 1/2] iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind Vasant Hegde
@ 2023-06-09 10:51 ` Vasant Hegde
2023-07-05 4:35 ` [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues Vasant Hegde
2023-07-14 14:17 ` Joerg Roedel
3 siblings, 0 replies; 5+ messages in thread
From: Vasant Hegde @ 2023-06-09 10:51 UTC (permalink / raw)
To: iommu, joro; +Cc: suravee.suthikulpanit, jgg, dmarcovitch, Vasant Hegde
Clear pasid state in device amd_iommu_free_device() path. It will make
sure no new ppr notifier is registered in free path.
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
drivers/iommu/amd/iommu_v2.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
index 858748baae0b..1df1d227bd26 100644
--- a/drivers/iommu/amd/iommu_v2.c
+++ b/drivers/iommu/amd/iommu_v2.c
@@ -327,6 +327,9 @@ static void free_pasid_states(struct device_state *dev_state)
put_pasid_state(pasid_state);
+ /* Clear the pasid state so that the pasid can be re-used */
+ clear_pasid_state(dev_state, pasid_state->pasid);
+
/*
* This will call the mn_release function and
* unbind the PASID
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues
2023-06-09 10:51 [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues Vasant Hegde
2023-06-09 10:51 ` [PATCH v2 1/2] iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind Vasant Hegde
2023-06-09 10:51 ` [PATCH v2 2/2] iommu/amd/iommu_v2: Clear pasid state in free path Vasant Hegde
@ 2023-07-05 4:35 ` Vasant Hegde
2023-07-14 14:17 ` Joerg Roedel
3 siblings, 0 replies; 5+ messages in thread
From: Vasant Hegde @ 2023-07-05 4:35 UTC (permalink / raw)
To: iommu, joro; +Cc: suravee.suthikulpanit, jgg, dmarcovitch
HI Joerg,
On 6/9/2023 4:21 PM, Vasant Hegde wrote:
> This patch series fixes the refcount warning and pasid state in free
> path.
>
> V1 : https://lore.kernel.org/linux-iommu/20230124104355.119166-1-vasant.hegde@amd.com/
Ping. Did you get a chance to look into this series?
-Vasant
>
> @Jason, Joerg,
> During v1 series review Jason had suggested few things including
> moving to xarray to handle PASID. We are planning to implement
> xarray as separate series.
>
> This series fixes the kernel warning we are seeing because of
> commit 8bc54824da4e. I am keeping changes as minimal so that
> its easy to backport.
>
> Base commit: d11370bf64c57a (iommu/next branch)
>
> Daniel Marcovitch (1):
> iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind
>
> Vasant Hegde (1):
> iommu/amd/iommu_v2: Clear pasid state in free path
>
> drivers/iommu/amd/iommu_v2.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues
2023-06-09 10:51 [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues Vasant Hegde
` (2 preceding siblings ...)
2023-07-05 4:35 ` [PATCH v2 0/2] iommu/amd/iommu_v2: Fix refcount related issues Vasant Hegde
@ 2023-07-14 14:17 ` Joerg Roedel
3 siblings, 0 replies; 5+ messages in thread
From: Joerg Roedel @ 2023-07-14 14:17 UTC (permalink / raw)
To: Vasant Hegde; +Cc: iommu, suravee.suthikulpanit, jgg, dmarcovitch
On Fri, Jun 09, 2023 at 10:51:44AM +0000, Vasant Hegde wrote:
> Daniel Marcovitch (1):
> iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind
>
> Vasant Hegde (1):
> iommu/amd/iommu_v2: Clear pasid state in free path
>
> drivers/iommu/amd/iommu_v2.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread