From: "Koenig, Christian" <Christian.Koenig@amd.com>
To: Jason Gunthorpe <jgg@ziepe.ca>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
Jerome Glisse <jglisse@redhat.com>,
Ralph Campbell <rcampbell@nvidia.com>,
John Hubbard <jhubbard@nvidia.com>,
"Kuehling, Felix" <Felix.Kuehling@amd.com>
Cc: "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
"Deucher, Alexander" <Alexander.Deucher@amd.com>,
Ben Skeggs <bskeggs@redhat.com>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>,
"Zhou, David(ChunMing)" <David1.Zhou@amd.com>,
Dennis Dalessandro <dennis.dalessandro@intel.com>,
Juergen Gross <jgross@suse.com>,
Mike Marciniszyn <mike.marciniszyn@intel.com>,
Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>,
Petr Cvek <petrcvekcz@gmail.com>,
Stefano Stabellini <sstabellini@kernel.org>,
"nouveau@lists.freedesktop.org" <nouveau@lists.freedesktop.org>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
Christoph Hellwig <hch@infradead.org>,
Jason Gunthorpe <jgg@mellanox.com>
Subject: Re: [PATCH v2 07/15] drm/radeon: use mmu_range_notifier_insert
Date: Tue, 29 Oct 2019 07:48:09 +0000 [thread overview]
Message-ID: <e05cc822-bba8-a7f0-1b28-73ea10b9e52e@amd.com> (raw)
In-Reply-To: <20191028201032.6352-8-jgg@ziepe.ca>
Am 28.10.19 um 21:10 schrieb Jason Gunthorpe:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> The new API is an exact match for the needs of radeon.
>
> For some reason radeon tries to remove overlapping ranges from the
> interval tree, but interval trees (and mmu_range_notifier_insert)
> support overlapping ranges directly. Simply delete all this code.
>
> Since this driver is missing a invalidate_range_end callback, but
> still calls get_user_pages(), it cannot be correct against all races.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Petr Cvek <petrcvekcz@gmail.com>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/radeon/radeon.h | 9 +-
> drivers/gpu/drm/radeon/radeon_mn.c | 219 ++++++-----------------------
> 2 files changed, 52 insertions(+), 176 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index d59b004f669583..27959f3ace1152 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -68,6 +68,10 @@
> #include <linux/hashtable.h>
> #include <linux/dma-fence.h>
>
> +#ifdef CONFIG_MMU_NOTIFIER
> +#include <linux/mmu_notifier.h>
> +#endif
> +
> #include <drm/ttm/ttm_bo_api.h>
> #include <drm/ttm/ttm_bo_driver.h>
> #include <drm/ttm/ttm_placement.h>
> @@ -509,8 +513,9 @@ struct radeon_bo {
> struct ttm_bo_kmap_obj dma_buf_vmap;
> pid_t pid;
>
> - struct radeon_mn *mn;
> - struct list_head mn_list;
> +#ifdef CONFIG_MMU_NOTIFIER
> + struct mmu_range_notifier notifier;
> +#endif
> };
> #define gem_to_radeon_bo(gobj) container_of((gobj), struct radeon_bo, tbo.base)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
> index dbab9a3a969b9e..d3d41e20a64922 100644
> --- a/drivers/gpu/drm/radeon/radeon_mn.c
> +++ b/drivers/gpu/drm/radeon/radeon_mn.c
> @@ -36,131 +36,51 @@
>
> #include "radeon.h"
>
> -struct radeon_mn {
> - struct mmu_notifier mn;
> -
> - /* objects protected by lock */
> - struct mutex lock;
> - struct rb_root_cached objects;
> -};
> -
> -struct radeon_mn_node {
> - struct interval_tree_node it;
> - struct list_head bos;
> -};
> -
> /**
> - * radeon_mn_invalidate_range_start - callback to notify about mm change
> + * radeon_mn_invalidate - callback to notify about mm change
> *
> * @mn: our notifier
> - * @mn: the mm this callback is about
> - * @start: start of updated range
> - * @end: end of updated range
> + * @range: the VMA under invalidation
> *
> * We block for all BOs between start and end to be idle and
> * unmap them by move them into system domain again.
> */
> -static int radeon_mn_invalidate_range_start(struct mmu_notifier *mn,
> - const struct mmu_notifier_range *range)
> +static bool radeon_mn_invalidate(struct mmu_range_notifier *mn,
> + const struct mmu_notifier_range *range,
> + unsigned long cur_seq)
> {
> - struct radeon_mn *rmn = container_of(mn, struct radeon_mn, mn);
> + struct radeon_bo *bo = container_of(mn, struct radeon_bo, notifier);
> struct ttm_operation_ctx ctx = { false, false };
> - struct interval_tree_node *it;
> - unsigned long end;
> - int ret = 0;
> -
> - /* notification is exclusive, but interval is inclusive */
> - end = range->end - 1;
> -
> - /* TODO we should be able to split locking for interval tree and
> - * the tear down.
> - */
> - if (mmu_notifier_range_blockable(range))
> - mutex_lock(&rmn->lock);
> - else if (!mutex_trylock(&rmn->lock))
> - return -EAGAIN;
> -
> - it = interval_tree_iter_first(&rmn->objects, range->start, end);
> - while (it) {
> - struct radeon_mn_node *node;
> - struct radeon_bo *bo;
> - long r;
> -
> - if (!mmu_notifier_range_blockable(range)) {
> - ret = -EAGAIN;
> - goto out_unlock;
> - }
> -
> - node = container_of(it, struct radeon_mn_node, it);
> - it = interval_tree_iter_next(it, range->start, end);
> + long r;
>
> - list_for_each_entry(bo, &node->bos, mn_list) {
> + if (!bo->tbo.ttm || bo->tbo.ttm->state != tt_bound)
> + return true;
>
> - if (!bo->tbo.ttm || bo->tbo.ttm->state != tt_bound)
> - continue;
> + if (!mmu_notifier_range_blockable(range))
> + return false;
>
> - r = radeon_bo_reserve(bo, true);
> - if (r) {
> - DRM_ERROR("(%ld) failed to reserve user bo\n", r);
> - continue;
> - }
> -
> - r = dma_resv_wait_timeout_rcu(bo->tbo.base.resv,
> - true, false, MAX_SCHEDULE_TIMEOUT);
> - if (r <= 0)
> - DRM_ERROR("(%ld) failed to wait for user bo\n", r);
> -
> - radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU);
> - r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> - if (r)
> - DRM_ERROR("(%ld) failed to validate user bo\n", r);
> -
> - radeon_bo_unreserve(bo);
> - }
> + r = radeon_bo_reserve(bo, true);
> + if (r) {
> + DRM_ERROR("(%ld) failed to reserve user bo\n", r);
> + return true;
> }
> -
> -out_unlock:
> - mutex_unlock(&rmn->lock);
> -
> - return ret;
> -}
> -
> -static void radeon_mn_release(struct mmu_notifier *mn, struct mm_struct *mm)
> -{
> - struct mmu_notifier_range range = {
> - .mm = mm,
> - .start = 0,
> - .end = ULONG_MAX,
> - .flags = 0,
> - .event = MMU_NOTIFY_UNMAP,
> - };
> -
> - radeon_mn_invalidate_range_start(mn, &range);
> -}
> -
> -static struct mmu_notifier *radeon_mn_alloc_notifier(struct mm_struct *mm)
> -{
> - struct radeon_mn *rmn;
>
> - rmn = kzalloc(sizeof(*rmn), GFP_KERNEL);
> - if (!rmn)
> - return ERR_PTR(-ENOMEM);
> + r = dma_resv_wait_timeout_rcu(bo->tbo.base.resv, true, false,
> + MAX_SCHEDULE_TIMEOUT);
> + if (r <= 0)
> + DRM_ERROR("(%ld) failed to wait for user bo\n", r);
>
> - mutex_init(&rmn->lock);
> - rmn->objects = RB_ROOT_CACHED;
> - return &rmn->mn;
> -}
> + radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU);
> + r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> + if (r)
> + DRM_ERROR("(%ld) failed to validate user bo\n", r);
>
> -static void radeon_mn_free_notifier(struct mmu_notifier *mn)
> -{
> - kfree(container_of(mn, struct radeon_mn, mn));
> + radeon_bo_unreserve(bo);
> + return true;
> }
>
> -static const struct mmu_notifier_ops radeon_mn_ops = {
> - .release = radeon_mn_release,
> - .invalidate_range_start = radeon_mn_invalidate_range_start,
> - .alloc_notifier = radeon_mn_alloc_notifier,
> - .free_notifier = radeon_mn_free_notifier,
> +static const struct mmu_range_notifier_ops radeon_mn_ops = {
> + .invalidate = radeon_mn_invalidate,
> };
>
> /**
> @@ -174,51 +94,21 @@ static const struct mmu_notifier_ops radeon_mn_ops = {
> */
> int radeon_mn_register(struct radeon_bo *bo, unsigned long addr)
> {
> - unsigned long end = addr + radeon_bo_size(bo) - 1;
> - struct mmu_notifier *mn;
> - struct radeon_mn *rmn;
> - struct radeon_mn_node *node = NULL;
> - struct list_head bos;
> - struct interval_tree_node *it;
> -
> - mn = mmu_notifier_get(&radeon_mn_ops, current->mm);
> - if (IS_ERR(mn))
> - return PTR_ERR(mn);
> - rmn = container_of(mn, struct radeon_mn, mn);
> -
> - INIT_LIST_HEAD(&bos);
> -
> - mutex_lock(&rmn->lock);
> -
> - while ((it = interval_tree_iter_first(&rmn->objects, addr, end))) {
> - kfree(node);
> - node = container_of(it, struct radeon_mn_node, it);
> - interval_tree_remove(&node->it, &rmn->objects);
> - addr = min(it->start, addr);
> - end = max(it->last, end);
> - list_splice(&node->bos, &bos);
> - }
> -
> - if (!node) {
> - node = kmalloc(sizeof(struct radeon_mn_node), GFP_KERNEL);
> - if (!node) {
> - mutex_unlock(&rmn->lock);
> - return -ENOMEM;
> - }
> - }
> -
> - bo->mn = rmn;
> -
> - node->it.start = addr;
> - node->it.last = end;
> - INIT_LIST_HEAD(&node->bos);
> - list_splice(&bos, &node->bos);
> - list_add(&bo->mn_list, &node->bos);
> -
> - interval_tree_insert(&node->it, &rmn->objects);
> -
> - mutex_unlock(&rmn->lock);
> -
> + int ret;
> +
> + bo->notifier.ops = &radeon_mn_ops;
> + ret = mmu_range_notifier_insert(&bo->notifier, addr, radeon_bo_size(bo),
> + current->mm);
> + if (ret)
> + return ret;
> +
> + /*
> + * FIXME: radeon appears to allow get_user_pages to run during
> + * invalidate_range_start/end, which is not a safe way to read the
> + * PTEs. It should use the mmu_range_read_begin() scheme around the
> + * get_user_pages to ensure that the PTEs are read properly
> + */
> + mmu_range_read_begin(&bo->notifier);
> return 0;
> }
>
> @@ -231,27 +121,8 @@ int radeon_mn_register(struct radeon_bo *bo, unsigned long addr)
> */
> void radeon_mn_unregister(struct radeon_bo *bo)
> {
> - struct radeon_mn *rmn = bo->mn;
> - struct list_head *head;
> -
> - if (!rmn)
> + if (!bo->notifier.mm)
> return;
> -
> - mutex_lock(&rmn->lock);
> - /* save the next list entry for later */
> - head = bo->mn_list.next;
> -
> - list_del(&bo->mn_list);
> -
> - if (list_empty(head)) {
> - struct radeon_mn_node *node;
> - node = container_of(head, struct radeon_mn_node, bos);
> - interval_tree_remove(&node->it, &rmn->objects);
> - kfree(node);
> - }
> -
> - mutex_unlock(&rmn->lock);
> -
> - mmu_notifier_put(&rmn->mn);
> - bo->mn = NULL;
> + mmu_range_notifier_remove(&bo->notifier);
> + bo->notifier.mm = NULL;
> }
next prev parent reply other threads:[~2019-10-29 7:48 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-28 20:10 [PATCH v2 00/15] Consolidate the mmu notifier interval_tree and locking Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 01/15] mm/mmu_notifier: define the header pre-processor parts even if disabled Jason Gunthorpe
2019-11-05 21:23 ` John Hubbard
2019-11-06 13:36 ` Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier Jason Gunthorpe
2019-10-29 22:04 ` Kuehling, Felix
2019-10-29 22:56 ` Jason Gunthorpe
2019-11-07 0:23 ` John Hubbard
2019-11-07 2:08 ` Jerome Glisse
2019-11-07 20:11 ` Jason Gunthorpe
2019-11-07 21:04 ` Jerome Glisse
2019-11-08 0:32 ` Jason Gunthorpe
2019-11-08 2:00 ` Jerome Glisse
2019-11-08 20:19 ` Jason Gunthorpe
2019-11-07 20:06 ` Jason Gunthorpe
2019-11-07 20:53 ` John Hubbard
2019-11-08 15:26 ` Jason Gunthorpe
2019-11-08 6:33 ` Christoph Hellwig
2019-11-08 13:43 ` Jerome Glisse
2019-10-28 20:10 ` [PATCH v2 03/15] mm/hmm: allow hmm_range to be used with a mmu_range_notifier or hmm_mirror Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 04/15] mm/hmm: define the pre-processor related parts of hmm.h even if disabled Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 05/15] RDMA/odp: Use mmu_range_notifier_insert() Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 06/15] RDMA/hfi1: Use mmu_range_notifier_inset for user_exp_rcv Jason Gunthorpe
2019-10-29 12:19 ` Dennis Dalessandro
2019-10-29 12:51 ` Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 07/15] drm/radeon: use mmu_range_notifier_insert Jason Gunthorpe
2019-10-29 7:48 ` Koenig, Christian [this message]
2019-10-28 20:10 ` [PATCH v2 08/15] xen/gntdev: Use select for DMA_SHARED_BUFFER Jason Gunthorpe
2019-11-01 18:26 ` Jason Gunthorpe
2019-11-05 14:44 ` Jürgen Groß
2019-11-07 9:39 ` Jürgen Groß
2019-10-28 20:10 ` [PATCH v2 09/15] xen/gntdev: use mmu_range_notifier_insert Jason Gunthorpe
2019-10-30 16:55 ` Boris Ostrovsky
2019-11-01 17:48 ` Jason Gunthorpe
2019-11-01 18:51 ` Boris Ostrovsky
2019-11-01 19:17 ` Jason Gunthorpe
2019-11-04 22:03 ` Boris Ostrovsky
2019-11-05 2:31 ` Jason Gunthorpe
2019-11-05 15:16 ` Boris Ostrovsky
2019-11-07 20:36 ` Jason Gunthorpe
2019-11-07 22:54 ` Boris Ostrovsky
2019-11-08 14:53 ` Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 10/15] nouveau: use mmu_notifier directly for invalidate_range_start Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 11/15] nouveau: use mmu_range_notifier instead of hmm_mirror Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 13/15] drm/amdgpu: Use mmu_range_insert " Jason Gunthorpe
2019-10-29 7:51 ` Koenig, Christian
2019-10-29 13:59 ` Jason Gunthorpe
2019-10-29 22:14 ` Kuehling, Felix
2019-10-29 23:09 ` Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier " Jason Gunthorpe
2019-10-29 19:22 ` Yang, Philip
2019-10-29 19:25 ` Jason Gunthorpe
2019-11-01 14:44 ` Yang, Philip
2019-11-01 15:12 ` Jason Gunthorpe
2019-11-01 15:59 ` Yang, Philip
2019-11-01 17:42 ` Jason Gunthorpe
2019-11-01 19:19 ` Jason Gunthorpe
2019-11-01 19:45 ` Yang, Philip
2019-11-01 19:50 ` Yang, Philip
2019-11-01 19:51 ` Jason Gunthorpe
2019-11-01 18:21 ` Jason Gunthorpe
2019-11-01 18:34 ` [PATCH v2a " Jason Gunthorpe
2019-10-28 20:10 ` [PATCH v2 15/15] mm/hmm: remove hmm_mirror and related Jason Gunthorpe
[not found] ` <20191028201032.6352-13-jgg@ziepe.ca>
2019-10-29 7:49 ` [PATCH v2 12/15] drm/amdgpu: Call find_vma under mmap_sem Koenig, Christian
2019-10-29 16:28 ` Kuehling, Felix
2019-10-29 13:07 ` Christian König
2019-10-29 17:19 ` Jason Gunthorpe
2019-11-01 19:54 ` [PATCH v2 00/15] Consolidate the mmu notifier interval_tree and locking Jason Gunthorpe
2019-11-01 20:54 ` Ralph Campbell
2019-11-04 20:40 ` Jason Gunthorpe
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=e05cc822-bba8-a7f0-1b28-73ea10b9e52e@amd.com \
--to=christian.koenig@amd.com \
--cc=Alexander.Deucher@amd.com \
--cc=David1.Zhou@amd.com \
--cc=Felix.Kuehling@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=boris.ostrovsky@oracle.com \
--cc=bskeggs@redhat.com \
--cc=dennis.dalessandro@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=hch@infradead.org \
--cc=jgg@mellanox.com \
--cc=jgg@ziepe.ca \
--cc=jglisse@redhat.com \
--cc=jgross@suse.com \
--cc=jhubbard@nvidia.com \
--cc=linux-mm@kvack.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mike.marciniszyn@intel.com \
--cc=nouveau@lists.freedesktop.org \
--cc=oleksandr_andrushchenko@epam.com \
--cc=petrcvekcz@gmail.com \
--cc=rcampbell@nvidia.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/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).