Re: [PATCH V2 3/4] iommufd: Add IOMMU_IOAS_CHANGE_PROCESS

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Oct 04, 2024 at 03:43:32PM -0400, Steven Sistare wrote:
> On 10/2/2024 1:51 PM, Jason Gunthorpe wrote:
> > On Thu, Sep 26, 2024 at 06:53:47AM -0700, Steve Sistare wrote:
> > 
> > > +static int charge_current(unsigned long *npinned)
> > > +{
> > > +	struct iopt_pages tmp = {
> > > +		.source_mm = current->mm,
> > > +		.source_task = current->group_leader,
> > > +		.source_user = current_user(),
> > > +	};
> > > +	unsigned int account_mode;
> > > +	int rc;
> > 
> > This seems like it needs a bit more protection, we shouldn't be
> > touching pages that are not set to current otherwise it will corrupt
> > the counts during the uncharge?
> > 
> > Maybe the whole operation should fail if any areas are not owned by
> > this process?
> 
> I don't follow.  In a typical scenario, none of the areas are owned by the
> current process when IOMMU_IOAS_CHANGE_PROCESS is called.

Oh I see, I didn't quite remember this all correctly. It is OK then,
you are taking ownership of all ares to the current process and it
does correctly de-account for any prior area owner.

> be more efficient, albeit in a less traveled error path.  I'll add it for
> symmetry.

Yeah symmetry.

Jason