On Wed, Aug 21, 2019 at 04:47:47PM +0000, Michal Kalderon wrote: > > 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. This already added a callback to free the xa_entry, why can't it free all the memory too when kref goes to 0? Jason