From: Gal Pressman <galpress@amazon.com>
To: Michal Kalderon <michal.kalderon@marvell.com>
Cc: <mkalderon@marvell.com>, <aelior@marvell.com>, <jgg@ziepe.ca>,
<dledford@redhat.com>, <bmt@zurich.ibm.com>, <sleybo@amazon.com>,
<leon@kernel.org>, <linux-rdma@vger.kernel.org>,
Ariel Elior <ariel.elior@marvell.com>
Subject: Re: [PATCH v8 rdma-next 2/7] RDMA/core: Create mmap database and cookie helper functions
Date: Thu, 29 Aug 2019 14:35:45 +0300 [thread overview]
Message-ID: <4d80bab8-d48c-70b3-52ba-494c98e8a349@amazon.com> (raw)
In-Reply-To: <20190827132846.9142-3-michal.kalderon@marvell.com>
On 27/08/2019 16:28, Michal Kalderon wrote:
> +/**
> + * 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.
> + * @len: the length the user wants 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. The function should
> + * be called only once per mmap. It initializes the vma and
> + * increases the entries ref-count. Once the memory is unmapped
> + * the ref-count will decrease. When the refcount reaches zero
> + * the entry will be deleted.
> + *
> + * 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, u64 len,
> + struct vm_area_struct *vma)
> +{
> + struct rdma_user_mmap_entry *entry;
> + u64 mmap_page;
> +
> + mmap_page = key >> PAGE_SHIFT;
> + if (mmap_page > U32_MAX)
> + return NULL;
> +
> + entry = xa_load(&ucontext->mmap_xa, mmap_page);
> + if (!entry)
> + return NULL;
I'm probably missing something, what happens if an insertion is done, a get is
called and right at this point (before kref_get) the entry is being removed (and
freed by the driver)?
> +
> + kref_get(&entry->ref);
> + rdma_umap_priv_init(vma, entry);
> +
> + ibdev_dbg(ucontext->device,
> + "mmap: key[%#llx] npages[%#x] returned\n",
> + key, entry->npages);
> +
> + return entry;
> +}
> +EXPORT_SYMBOL(rdma_user_mmap_entry_get);
> +
> +void rdma_user_mmap_entry_free(struct kref *kref)
> +{
> + struct rdma_user_mmap_entry *entry =
> + container_of(kref, struct rdma_user_mmap_entry, ref);
> + struct ib_ucontext *ucontext = entry->ucontext;
> + unsigned long i;
> +
> + /* need to erase all entries occupied... */
To be clear, this erases all pages of a single entry right? The comment is
confusing.
> + for (i = 0; i < entry->npages; i++) {
> + xa_erase(&ucontext->mmap_xa, entry->mmap_page + i);
> +
> + ibdev_dbg(ucontext->device,
> + "mmap: key[%#llx] npages[%#x] removed\n",
> + rdma_user_mmap_get_key(entry),
> + entry->npages);
This print can be outside of the loop, it doesn't change between iterations.
> + }
> + if (ucontext->device->ops.mmap_free)
> + ucontext->device->ops.mmap_free(entry);
> +}
> +/**
> + * rdma_user_mmap_entry_insert() - Allocate and 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 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 retrieve information such as address to
> + * be mapped and how.
> + *
> + * Return: unique key or RDMA_USER_MMAP_INVALID if entry was not added.
> + */
> +u64 rdma_user_mmap_entry_insert(struct ib_ucontext *ucontext,
> + struct rdma_user_mmap_entry *entry,
> + u64 length)
> +{
> + struct ib_uverbs_file *ufile = ucontext->ufile;
> + XA_STATE(xas, &ucontext->mmap_xa, 0);
> + u32 xa_first, xa_last, npages;
> + int err, i;
> +
> + if (!entry)
> + return RDMA_USER_MMAP_INVALID;
> +
> + kref_init(&entry->ref);
> + entry->ucontext = ucontext;
> +
> + /* We want the whole allocation to be done without interruption
> + * from a different thread. The allocation requires finding a
> + * free range and storing. During the xa_insert the lock could be
> + * released, we don't want another thread taking the gap.
> + */
> + mutex_lock(&ufile->umap_lock);
> +
> + xa_lock(&ucontext->mmap_xa);
Doesn't the mutex replace the xa_lock?
> +
> + /* We want to find an empty range */
> + npages = (u32)DIV_ROUND_UP(length, PAGE_SIZE);
> + entry->npages = npages;
> + do {
> + /* First find an empty index */
> + xas_find_marked(&xas, U32_MAX, XA_FREE_MARK);
> + if (xas.xa_node == XAS_RESTART)
> + goto err_unlock;
> +
> + xa_first = xas.xa_index;
> +
> + /* Is there enough room to have the range? */
> + if (check_add_overflow(xa_first, npages, &xa_last))
> + goto err_unlock;
> +
> + /* Now look for the next present entry. If such doesn't
> + * exist, we found an empty range and can proceed
> + */
> + xas_next_entry(&xas, xa_last - 1);
> + if (xas.xa_node == XAS_BOUNDS || xas.xa_index >= xa_last)
> + break;
> + /* o/w look for the next free entry */
> + } while (true);
> +
> + for (i = xa_first; i < xa_last; i++) {
> + err = __xa_insert(&ucontext->mmap_xa, i, entry, GFP_KERNEL);
> + if (err)
> + goto err_undo;
> + }
> +
> + entry->mmap_page = xa_first;
> + xa_unlock(&ucontext->mmap_xa);
> +
> + mutex_unlock(&ufile->umap_lock);
> + ibdev_dbg(ucontext->device,
> + "mmap: key[%#llx] npages[%#x] inserted\n",
> + rdma_user_mmap_get_key(entry), npages);
> +
> + return rdma_user_mmap_get_key(entry);
> +
> +err_undo:
> + for (; i > xa_first; i--)
> + __xa_erase(&ucontext->mmap_xa, i - 1);
> +
> +err_unlock:
> + xa_unlock(&ucontext->mmap_xa);
> + mutex_unlock(&ufile->umap_lock);
> + return RDMA_USER_MMAP_INVALID;
> +}
next prev parent reply other threads:[~2019-08-29 11:36 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-27 13:28 [PATCH v8 rdma-next 0/7] RDMA/qedr: Use the doorbell overflow recovery mechanism for RDMA Michal Kalderon
2019-08-27 13:28 ` [PATCH v8 rdma-next 1/7] RDMA/core: Move core content from ib_uverbs to ib_core Michal Kalderon
2019-08-27 13:28 ` [PATCH v8 rdma-next 2/7] RDMA/core: Create mmap database and cookie helper functions Michal Kalderon
2019-08-29 11:35 ` Gal Pressman [this message]
2019-08-29 12:40 ` Jason Gunthorpe
2019-08-30 6:10 ` [EXT] " Michal Kalderon
2019-08-27 13:28 ` [PATCH v8 rdma-next 3/7] RDMA/efa: Use the common mmap_xa helpers Michal Kalderon
2019-08-29 14:20 ` Gal Pressman
2019-08-30 6:15 ` Michal Kalderon
2019-09-01 6:56 ` Gal Pressman
2019-09-02 7:47 ` Michal Kalderon
2019-08-27 13:28 ` [PATCH v8 rdma-next 4/7] RDMA/siw: " Michal Kalderon
2019-08-27 13:28 ` [PATCH v8 rdma-next 5/7] RDMA/qedr: Use the common mmap API Michal Kalderon
2019-08-27 13:28 ` [PATCH v8 rdma-next 6/7] RDMA/qedr: Add doorbell overflow recovery support Michal Kalderon
2019-08-27 13:28 ` [PATCH v8 rdma-next 7/7] RDMA/qedr: Add iWARP doorbell " Michal Kalderon
2019-08-28 14:41 ` [PATCH v8 rdma-next 0/7] RDMA/qedr: Use the doorbell overflow recovery mechanism for RDMA Gal Pressman
2019-08-30 12:07 ` [PATCH v8 rdma-next 4/7] RDMA/siw: Use the common mmap_xa helpers Bernard Metzler
2019-08-30 12:42 ` [EXT] " Michal Kalderon
2019-08-30 13:19 ` Bernard Metzler
2019-09-02 8:00 ` Michal Kalderon
2019-09-02 10:55 ` Bernard Metzler
2019-09-02 11:21 ` Michal Kalderon
2019-09-02 11:18 ` Bernard Metzler
2019-09-02 13:16 ` Michal Kalderon
2019-09-02 14:17 ` Bernard Metzler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4d80bab8-d48c-70b3-52ba-494c98e8a349@amazon.com \
--to=galpress@amazon.com \
--cc=aelior@marvell.com \
--cc=ariel.elior@marvell.com \
--cc=bmt@zurich.ibm.com \
--cc=dledford@redhat.com \
--cc=jgg@ziepe.ca \
--cc=leon@kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=michal.kalderon@marvell.com \
--cc=mkalderon@marvell.com \
--cc=sleybo@amazon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).