On Wed, Oct 30, 2019 at 11:44:11AM +0200, Michal Kalderon wrote:
> diff --git a/drivers/infiniband/core/ib_core_uverbs.c b/drivers/infiniband/core/ib_core_uverbs.c
> index b74d2a2fb342..1ffc89fd5d94 100644
> +++ b/drivers/infiniband/core/ib_core_uverbs.c
> @@ -71,3 +71,204 @@ int rdma_user_mmap_io(struct ib_ucontext *ucontext, struct vm_area_struct *vma,
> return 0;
> }
> EXPORT_SYMBOL(rdma_user_mmap_io);
> +
> +/**
> + * rdma_user_mmap_entry_get() - Get an entry from the mmap_xa.
> + *
> + * @ucontext: associated user context.
> + * @key: the key received from rdma_user_mmap_entry_insert which
> + * is provided by user as the address to map.
> + * @vma: the vma related to the current mmap call.
> + *
> + * This function is called when a user tries to mmap a key it
> + * initially received from the driver. The key was created by
> + * the function rdma_user_mmap_entry_insert.
> + * This function increases the refcnt of the entry so that it won't
> + * be deleted from the xa in the meantime.
> + *
> + * Return an entry if exists or NULL if there is no match.
> + */
> +struct rdma_user_mmap_entry *
> +rdma_user_mmap_entry_get(struct ib_ucontext *ucontext, u64 key,
> + struct vm_area_struct *vma)
This should just accept 'key' (but called pgoff for clarity), but
since everyone has a vma a static inline wrapper should take in the
vma
> +{
> + struct rdma_user_mmap_entry *entry;
> + u64 mmap_page;
> +
> + mmap_page = key >> PAGE_SHIFT;
It isn't even really 'key' here if it was shifted as that is called
'offset'
> + if (mmap_page > U32_MAX)
> + return NULL;
> +
> + xa_lock(&ucontext->mmap_xa);
> +
> + entry = xa_load(&ucontext->mmap_xa, mmap_page);
Since each xarray entry in the range stores the same pointer this
needs to check that mmap_page is actually the right entry, attempting
to directly mmap the 2nd page should fail as we don't have the rest of
the infrastructure to make that work.
> +/**
> + * rdma_user_mmap_entry_put() - Drop reference to the mmap entry
> + *
> + * @ucontext: associated user context.
> + * @entry: an entry in the mmap_xa.
> + *
> + * This function is called when the mapping is closed if it was
> + * an io mapping or when the driver is done with the entry for
> + * some other reason.
> + * Should be called after rdma_user_mmap_entry_get was called
> + * and entry is no longer needed. This function will erase the
> + * entry and free it if its refcnt reaches zero.
> + */
> +void rdma_user_mmap_entry_put(struct ib_ucontext *ucontext,
> + struct rdma_user_mmap_entry *entry)
> +{
ucontext is not needed
> + kref_put(&entry->ref, rdma_user_mmap_entry_free);
> +}
> +EXPORT_SYMBOL(rdma_user_mmap_entry_put);
> +
> +/**
> + * rdma_user_mmap_entry_remove() - Drop reference to entry and
> + * mark it as invalid.
> + *
> + * @ucontext: associated user context.
> + * @entry: the entry to insert into the mmap_xa
> + */
> +void rdma_user_mmap_entry_remove(struct ib_ucontext *ucontext,
> + struct rdma_user_mmap_entry *entry)
> +{
ucontext is not needed
> +/**
> + * rdma_user_mmap_entry_insert() - Insert an entry to the mmap_xa.
> + *
> + * @ucontext: associated user context.
> + * @entry: the entry to insert into the mmap_xa
> + * @length: length of the address that will be mmapped
> + *
> + * 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 calls mmap with the given key.
> + * The function allocates a unique key that should be provided to user, the user
> + * will use the key to retrieve information such as address to
> + * be mapped and how.
> + *
> + * Return: 0 on success and -ENOMEM on failure
> + */
> +int rdma_user_mmap_entry_insert(struct ib_ucontext *ucontext,
> + struct rdma_user_mmap_entry *entry,
> + size_t length)
> +{
> + struct ib_uverbs_file *ufile = ucontext->ufile;
> + XA_STATE(xas, &ucontext->mmap_xa, 0);
> + u32 xa_first, xa_last, npages;
> + int err, i;
'i' should be u32
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 6a47ba85c54c..8a87c9d442bc 100644
> +++ b/include/rdma/ib_verbs.h
> @@ -1471,6 +1471,7 @@ struct ib_ucontext {
> * Implementation details of the RDMA core, don't use in drivers:
> */
> struct rdma_restrack_entry res;
> + struct xarray mmap_xa;
> };
>
> struct ib_uobject {
> @@ -2251,6 +2252,20 @@ struct iw_cm_conn_param;
>
> #define DECLARE_RDMA_OBJ_SIZE(ib_struct) size_t size_##ib_struct
>
> +struct rdma_user_mmap_entry {
> + struct kref ref;
> + struct ib_ucontext *ucontext;
> + u32 npages;
> + u32 mmap_page;
> + bool invalid;
These names are confusing, lets use:
unsigned long start_pgoff;
size_t npages;
bool driver_removed;
> +};
> +
> +static inline u64
> +rdma_user_mmap_get_key(const struct rdma_user_mmap_entry *entry)
> +{
> + return (u64)entry->mmap_page << PAGE_SHIFT;
> +}
This is offset not key, in fact lets not use the word 'key' at
all. Either 'pgoff' or 'offset'
There are also a number of fixables in comments, grammer, indentation
and style..
Jason
