On Thu, Oct 07, 2021 at 12:23:13PM +1100, David Gibson wrote: > On Fri, Oct 01, 2021 at 09:43:22AM -0300, Jason Gunthorpe wrote: > > On Thu, Sep 30, 2021 at 01:10:29PM +1000, David Gibson wrote: > > > On Wed, Sep 29, 2021 at 09:24:57AM -0300, Jason Gunthorpe wrote: > > > > On Wed, Sep 29, 2021 at 03:25:54PM +1000, David Gibson wrote: > > > > > > > > > > +struct iommufd_device { > > > > > > + unsigned int id; > > > > > > + struct iommufd_ctx *ictx; > > > > > > + struct device *dev; /* always be the physical device */ > > > > > > + u64 dev_cookie; > > > > > > > > > > Why do you need both an 'id' and a 'dev_cookie'? Since they're both > > > > > unique, couldn't you just use the cookie directly as the index into > > > > > the xarray? > > > > > > > > ID is the kernel value in the xarray - xarray is much more efficient & > > > > safe with small kernel controlled values. > > > > > > > > dev_cookie is a user assigned value that may not be unique. It's > > > > purpose is to allow userspace to receive and event and go back to its > > > > structure. Most likely userspace will store a pointer here, but it is > > > > also possible userspace could not use it. > > > > > > > > It is a pretty normal pattern > > > > > > Hm, ok. Could you point me at an example? > > > > For instance user_data vs fd in io_uring > > Ok, but one of those is an fd, which is an existing type of handle. > Here we're introducing two different unique handles that aren't an > existing kernel concept. I'm not sure how that matters, the kernel has many handles - and we get to make more of them.. Look at xarray/idr users in the kernel, many of those are making userspace handles. > That said... is there any strong reason why user_data needs to be > unique? I can imagine userspace applications where you don't care > which device the notification is coming from - or at least don't care > down to the same granularity that /dev/iommu is using. In which case > having the kernel provided unique handle and the > not-necessarily-unique user_data would make perfect sense. I don't think the user_data 64 bit value should be unique, it is just transported from user to kernal and back again. It is *not* a handle, it is a cookie. Handles for the kernel/user boundary should come from xarrays that have nice lookup properties - not from user provided 64 bit values that have to be stored in red black trees.. Jason