From: Jason Gunthorpe <jgg@nvidia.com>
To: Vasant Hegde <vasant.hegde@amd.com>
Cc: iommu@lists.linux.dev, joro@8bytes.org,
suravee.suthikulpanit@amd.com,
Daniel Marcovitch <dmarcovitch@nvidia.com>
Subject: Re: [PATCH 1/3] iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind
Date: Fri, 27 Jan 2023 08:58:10 -0400 [thread overview]
Message-ID: <Y9PKYqdxYevgFbNq@nvidia.com> (raw)
In-Reply-To: <6fa315a8-844f-19eb-1a23-41cf8b51313a@amd.com>
On Fri, Jan 27, 2023 at 10:14:15AM +0530, Vasant Hegde wrote:
> Hi Jason,
>
>
> On 1/24/2023 8:53 PM, Jason Gunthorpe wrote:
> > On Tue, Jan 24, 2023 at 10:43:53AM +0000, Vasant Hegde wrote:
> >> 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
> >
> > I've no objection to this quick fix
> >
> > But the issue here is using a refcount for something that is not a
> > refcount. This usage model is should just be a normal atomic if it is
> > only counting the number of notifications in progress.
>
> Originally it was using atomic counter.. which got converted to refcount by
> below commit:
>
> commit 8bc54824da4e8fcf0ed679cf09ac32f23d83254a
> Author: Xiyu Yang via iommu <iommu@lists.linux-foundation.org>
> Date: Mon Jul 19 16:32:58 2021 +0800
>
> iommu/amd: Convert from atomic_t to refcount_t on pasid_state->count
>
> .. after this commit it started throwing warnings. We thought of fixing the
> warning instead of reverting above commit.
The mechanical clean up was wrong because it subtly wasn't a refcount
:)
But given the other problems here you can fix it with the approach I gave
in the other email
Jason
next prev parent reply other threads:[~2023-01-27 12:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-24 10:43 [PATCH 0/3] iommu/amd/iommu_v2: Fix refcount related issues Vasant Hegde
2023-01-24 10:43 ` [PATCH 1/3] iommu/amd/iommu_v2: Fix pasid_state refcount dec hit 0 warning on pasid unbind Vasant Hegde
2023-01-24 15:23 ` Jason Gunthorpe
2023-01-27 4:44 ` Vasant Hegde
2023-01-27 12:58 ` Jason Gunthorpe [this message]
2023-01-24 10:43 ` [PATCH 2/3] iommu/amd/iommu_v2: Fix pasid_state->wq race Vasant Hegde
2023-01-24 15:30 ` Jason Gunthorpe
2023-01-24 10:43 ` [PATCH 3/3] iommu/amd/iommu_v2: Prevent scheduling new ppr notifier during unbind_pasid Vasant Hegde
2023-01-24 15:33 ` Jason Gunthorpe
2023-01-27 4:58 ` Vasant Hegde
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y9PKYqdxYevgFbNq@nvidia.com \
--to=jgg@nvidia.com \
--cc=dmarcovitch@nvidia.com \
--cc=iommu@lists.linux.dev \
--cc=joro@8bytes.org \
--cc=suravee.suthikulpanit@amd.com \
--cc=vasant.hegde@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.