[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180622152444.GC10465@dhcp22.suse.cz>
Date: Fri, 22 Jun 2018 17:24:44 +0200
From: Michal Hocko <mhocko@...nel.org>
To: Christian König <christian.koenig@....com>
Cc: LKML <linux-kernel@...r.kernel.org>,
"David (ChunMing) Zhou" <David1.Zhou@....com>,
Paolo Bonzini <pbonzini@...hat.com>,
Radim Krčmář <rkrcmar@...hat.com>,
Alex Deucher <alexander.deucher@....com>,
David Airlie <airlied@...ux.ie>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Doug Ledford <dledford@...hat.com>,
Jason Gunthorpe <jgg@...pe.ca>,
Mike Marciniszyn <mike.marciniszyn@...el.com>,
Dennis Dalessandro <dennis.dalessandro@...el.com>,
Sudeep Dutt <sudeep.dutt@...el.com>,
Ashutosh Dixit <ashutosh.dixit@...el.com>,
Dimitri Sivanich <sivanich@....com>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Juergen Gross <jgross@...e.com>,
Jérôme Glisse <jglisse@...hat.com>,
Andrea Arcangeli <aarcange@...hat.com>, kvm@...r.kernel.org,
amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
intel-gfx@...ts.freedesktop.org, linux-rdma@...r.kernel.org,
xen-devel@...ts.xenproject.org, linux-mm@...ck.org,
David Rientjes <rientjes@...gle.com>,
Felix Kuehling <felix.kuehling@....com>
Subject: Re: [RFC PATCH] mm, oom: distinguish blockable mode for mmu notifiers
On Fri 22-06-18 17:13:02, Christian König wrote:
> Hi Michal,
>
> [Adding Felix as well]
>
> Well first of all you have a misconception why at least the AMD graphics
> driver need to be able to sleep in an MMU notifier: We need to sleep because
> we need to wait for hardware operations to finish and *NOT* because we need
> to wait for locks.
>
> I'm not sure if your flag now means that you generally can't sleep in MMU
> notifiers any more, but if that's the case at least AMD hardware will break
> badly. In our case the approach of waiting for a short time for the process
> to be reaped and then select another victim actually sounds like the right
> thing to do.
Well, I do not need to make the notifier code non blocking all the time.
All I need is to ensure that it won't sleep if the flag says so and
return -EAGAIN instead.
So here is what I do for amdgpu:
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > index 83e344fbb50a..d138a526feff 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
> > @@ -136,12 +136,18 @@ void amdgpu_mn_unlock(struct amdgpu_mn *mn)
> > *
> > * Take the rmn read side lock.
> > */
> > -static void amdgpu_mn_read_lock(struct amdgpu_mn *rmn)
> > +static int amdgpu_mn_read_lock(struct amdgpu_mn *rmn, bool blockable)
> > {
> > - mutex_lock(&rmn->read_lock);
> > + if (blockable)
> > + mutex_lock(&rmn->read_lock);
> > + else if (!mutex_trylock(&rmn->read_lock))
> > + return -EAGAIN;
> > +
> > if (atomic_inc_return(&rmn->recursion) == 1)
> > down_read_non_owner(&rmn->lock);
> > mutex_unlock(&rmn->read_lock);
> > +
> > + return 0;
> > }
> > /**
> > @@ -197,10 +203,11 @@ static void amdgpu_mn_invalidate_node(struct amdgpu_mn_node *node,
> > * We block for all BOs between start and end to be idle and
> > * unmap them by move them into system domain again.
> > */
> > -static void amdgpu_mn_invalidate_range_start_gfx(struct mmu_notifier *mn,
> > +static int amdgpu_mn_invalidate_range_start_gfx(struct mmu_notifier *mn,
> > struct mm_struct *mm,
> > unsigned long start,
> > - unsigned long end)
> > + unsigned long end,
> > + bool blockable)
> > {
> > struct amdgpu_mn *rmn = container_of(mn, struct amdgpu_mn, mn);
> > struct interval_tree_node *it;
> > @@ -208,7 +215,11 @@ static void amdgpu_mn_invalidate_range_start_gfx(struct mmu_notifier *mn,
> > /* notification is exclusive, but interval is inclusive */
> > end -= 1;
> > - amdgpu_mn_read_lock(rmn);
> > + /* TODO we should be able to split locking for interval tree and
> > + * amdgpu_mn_invalidate_node
> > + */
> > + if (amdgpu_mn_read_lock(rmn, blockable))
> > + return -EAGAIN;
> > it = interval_tree_iter_first(&rmn->objects, start, end);
> > while (it) {
> > @@ -219,6 +230,8 @@ static void amdgpu_mn_invalidate_range_start_gfx(struct mmu_notifier *mn,
> > amdgpu_mn_invalidate_node(node, start, end);
> > }
> > +
> > + return 0;
> > }
> > /**
> > @@ -233,10 +246,11 @@ static void amdgpu_mn_invalidate_range_start_gfx(struct mmu_notifier *mn,
> > * necessitates evicting all user-mode queues of the process. The BOs
> > * are restorted in amdgpu_mn_invalidate_range_end_hsa.
> > */
> > -static void amdgpu_mn_invalidate_range_start_hsa(struct mmu_notifier *mn,
> > +static int amdgpu_mn_invalidate_range_start_hsa(struct mmu_notifier *mn,
> > struct mm_struct *mm,
> > unsigned long start,
> > - unsigned long end)
> > + unsigned long end,
> > + bool blockable)
> > {
> > struct amdgpu_mn *rmn = container_of(mn, struct amdgpu_mn, mn);
> > struct interval_tree_node *it;
> > @@ -244,7 +258,8 @@ static void amdgpu_mn_invalidate_range_start_hsa(struct mmu_notifier *mn,
> > /* notification is exclusive, but interval is inclusive */
> > end -= 1;
> > - amdgpu_mn_read_lock(rmn);
> > + if (amdgpu_mn_read_lock(rmn, blockable))
> > + return -EAGAIN;
> > it = interval_tree_iter_first(&rmn->objects, start, end);
> > while (it) {
> > @@ -262,6 +277,8 @@ static void amdgpu_mn_invalidate_range_start_hsa(struct mmu_notifier *mn,
> > amdgpu_amdkfd_evict_userptr(mem, mm);
> > }
> > }
> > +
> > + return 0;
> > }
> > /**
--
Michal Hocko
SUSE Labs
Powered by blists - more mailing lists