> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Michal Kalderon
>
> > From: Jason Gunthorpe <jgg@xxxxxxxx>
> > Sent: Tuesday, August 20, 2019 4:21 PM
> >
> > ----------------------------------------------------------------------
> > On Tue, Aug 20, 2019 at 03:18:42PM +0300, Michal Kalderon wrote:
> > > Create some common API's for adding entries to a xa_mmap.
> > > Searching for an entry and freeing one.
> > >
> > > +
> > > +/**
> > > + * rdma_user_mmap_entry_insert() - Allocate and insert an entry to
> > > +the
> > mmap_xa.
> > > + *
> > > + * @ucontext: associated user context.
> > > + * @obj: opaque driver object that will be stored in the entry.
> > > + * @address: The address that will be mmapped to the user
> > > + * @length: Length of the address that will be mmapped
> > > + * @mmap_flag: opaque driver flags related to the address (For
> > > + * example could be used for cachability)
> > > + *
> > > + * This function should be called by drivers that use the
> > > +rdma_user_mmap
> > > + * interface for handling user mmapped addresses. The database is
> > > +handled in
> > > + * the core and helper functions are provided to insert entries
> > > +into the
> > > + * database and extract entries when the user call mmap with the
> > > +given
> > key.
> > > + * The function returns a unique key that should be provided to
> > > +user, the user
> > > + * will use the key to map the given address.
> > > + *
> > > + * Return: unique key or RDMA_USER_MMAP_INVALID if entry was not
> > added.
> > > + */
> > > +u64 rdma_user_mmap_entry_insert(struct ib_ucontext *ucontext, void
> > *obj,
> > > + u64 address, u64 length, u8 mmap_flag) {
> > > + XA_STATE(xas, &ucontext->mmap_xa, 0);
> > > + struct rdma_user_mmap_entry *entry;
> > > + unsigned long index = 0, index_max;
> > > + u32 xa_first, xa_last, npages;
> > > + int err, i;
> > > + void *ent;
> > > +
> > > + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> > > + if (!entry)
> > > + return RDMA_USER_MMAP_INVALID;
> > > +
> > > + entry->obj = obj;
> >
> > It is more a kernel pattern to have the driver allocate a
> > rdma_user_mmap_entry and extend it with its 'priv', then use
> > container_of
> then would we also want the driver to free the memory ?
> Or will it be ok to free it using the kref put callback ?
Jason, I looked into this deeper today, it seems that since the
Core is the one handling the reference counting, and eventually
Freeing the object that it makes more sense to keep the allocation
In core and not in the drivers, since the driver won't be able to free
The entry without providing yet an additional callback function to the
Core to be called once the reference count reaches zero.
> >
> >
> > > +void rdma_user_mmap_entries_remove_free(struct ib_ucontext
> > *ucontext)
> > > +{
> > > + struct rdma_user_mmap_entry *entry;
> > > + unsigned long mmap_page;
> > > +
> > > + WARN_ON(!xa_empty(&ucontext->mmap_xa));
> > > + xa_for_each(&ucontext->mmap_xa, mmap_page, entry) {
> > > + ibdev_dbg(ucontext->device,
> > > + "mmap: obj[0x%p] key[%#llx] addr[%#llx] len[%#llx]
> > removed\n",
> > > + entry->obj, rdma_user_mmap_get_key(entry),
> > > + entry->address, entry->length);
> > > +
> > > + /* override the refcnt to make sure entry is deleted */
> > > + kref_init(&entry->ref);
> >
> > Yikes, no. The zap flow has to clean this up so the kref goes naturally to
> zero.
> Ok thanks
This actually turned out to be completely redundant, I'll leave the WARN_ON check only
And remove the function entirely, all entries should be removed prior to this, and if we
Reached here it means the refcnt won't go down to zero anyway and entries won't be
Removed. This will only happen with a bug.
Thanks,
Michal
