All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"Linux MM" <linux-mm@kvack.org>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Intel Graphics Development" <intel-gfx@lists.freedesktop.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Michal Hocko" <mhocko@suse.com>,
	"David Rientjes" <rientjes@google.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Jérôme Glisse" <jglisse@redhat.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>
Subject: Re: [PATCH 4/4] mm, notifier: Catch sleeping/blocking for !blockable
Date: Thu, 22 Aug 2019 16:27:18 +0200	[thread overview]
Message-ID: <CAKMK7uF5CMSUrG2jTYJ9M7tDK_Saxmxk6yLs62tfc-Ozj3p2BQ@mail.gmail.com> (raw)
In-Reply-To: <20190822142410.GB8339@ziepe.ca>

On Thu, Aug 22, 2019 at 4:24 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Aug 22, 2019 at 10:42:39AM +0200, Daniel Vetter wrote:
>
> > > RDMA has a mutex:
> > >
> > > ib_umem_notifier_invalidate_range_end
> > >   rbt_ib_umem_for_each_in_range
> > >    invalidate_range_start_trampoline
> > >     ib_umem_notifier_end_account
> > >       mutex_lock(&umem_odp->umem_mutex);
> > >
> > > I'm working to delete this path though!
> > >
> > > nonblocking or not follows the start, the same flag gets placed into
> > > the mmu_notifier_range struct passed to end.
> >
> > Ok, makes sense.
> >
> > I guess that also means the might_sleep (I started on that) in
> > invalidate_range_end also needs to be conditional? Or not bother with
> > a might_sleep in invalidate_range_end since you're working on removing
> > the last sleep in there?
>
> I might suggest the same pattern as used for locked, the might_sleep
> unconditionally on the start, and a 2nd might sleep after the IF in
> __mmu_notifier_invalidate_range_end()
>
> Observing that by audit all the callers already have the same locking
> context for start/end

My question was more about enforcing that going forward, since you're
working to remove all the sleeps from invalidate_range_end. I don't
want to add debug annotations which are stricter than what the other
side actually expects. But since currently there is still sleeping
locks in invalidate_range_end I think I'll just stick them in both
places. You can then (re)move it when the cleanup lands.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: "Michal Hocko" <mhocko@suse.com>,
	"Intel Graphics Development" <intel-gfx@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Linux MM" <linux-mm@kvack.org>,
	"Jérôme Glisse" <jglisse@redhat.com>,
	"David Rientjes" <rientjes@google.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH 4/4] mm, notifier: Catch sleeping/blocking for !blockable
Date: Thu, 22 Aug 2019 16:27:18 +0200	[thread overview]
Message-ID: <CAKMK7uF5CMSUrG2jTYJ9M7tDK_Saxmxk6yLs62tfc-Ozj3p2BQ@mail.gmail.com> (raw)
In-Reply-To: <20190822142410.GB8339@ziepe.ca>

On Thu, Aug 22, 2019 at 4:24 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Aug 22, 2019 at 10:42:39AM +0200, Daniel Vetter wrote:
>
> > > RDMA has a mutex:
> > >
> > > ib_umem_notifier_invalidate_range_end
> > >   rbt_ib_umem_for_each_in_range
> > >    invalidate_range_start_trampoline
> > >     ib_umem_notifier_end_account
> > >       mutex_lock(&umem_odp->umem_mutex);
> > >
> > > I'm working to delete this path though!
> > >
> > > nonblocking or not follows the start, the same flag gets placed into
> > > the mmu_notifier_range struct passed to end.
> >
> > Ok, makes sense.
> >
> > I guess that also means the might_sleep (I started on that) in
> > invalidate_range_end also needs to be conditional? Or not bother with
> > a might_sleep in invalidate_range_end since you're working on removing
> > the last sleep in there?
>
> I might suggest the same pattern as used for locked, the might_sleep
> unconditionally on the start, and a 2nd might sleep after the IF in
> __mmu_notifier_invalidate_range_end()
>
> Observing that by audit all the callers already have the same locking
> context for start/end

My question was more about enforcing that going forward, since you're
working to remove all the sleeps from invalidate_range_end. I don't
want to add debug annotations which are stricter than what the other
side actually expects. But since currently there is still sleeping
locks in invalidate_range_end I think I'll just stick them in both
places. You can then (re)move it when the cleanup lands.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-08-22 14:27 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-20  8:18 [PATCH 0/4] mmu notifier debug annotations/checks Daniel Vetter
2019-08-20  8:18 ` [PATCH 1/4] mm, notifier: Add a lockdep map for invalidate_range_start/end Daniel Vetter
2019-08-20 13:31   ` Jason Gunthorpe
2019-08-20  8:19 ` [PATCH 2/4] mm, notifier: Prime lockdep Daniel Vetter
2019-08-20 13:31   ` Jason Gunthorpe
2019-08-20  8:19 ` [PATCH 3/4] kernel.h: Add non_block_start/end() Daniel Vetter
2019-08-20  8:19   ` Daniel Vetter
2019-08-20 20:24   ` Daniel Vetter
2019-08-20 20:24     ` Daniel Vetter
2019-08-22 23:14     ` Andrew Morton
2019-08-22 23:14       ` Andrew Morton
2019-08-23  8:34       ` Daniel Vetter
2019-08-23  8:34         ` Daniel Vetter
2019-08-23 12:12         ` Jason Gunthorpe
2019-08-23 12:12           ` Jason Gunthorpe
2019-08-23 12:22           ` Peter Zijlstra
2019-08-23 12:22             ` Peter Zijlstra
2019-08-23 13:42           ` Daniel Vetter
2019-08-23 13:42             ` Daniel Vetter
2019-08-23 14:06             ` Peter Zijlstra
2019-08-23 14:06               ` Peter Zijlstra
2019-08-23 15:15               ` Daniel Vetter
2019-08-23 15:15                 ` Daniel Vetter
2019-08-23  8:48     ` Peter Zijlstra
2019-08-23  8:48       ` Peter Zijlstra
2019-08-20  8:19 ` [PATCH 4/4] mm, notifier: Catch sleeping/blocking for !blockable Daniel Vetter
2019-08-20 13:34   ` Jason Gunthorpe
2019-08-20 15:18     ` Daniel Vetter
2019-08-20 15:27       ` Jason Gunthorpe
2019-08-21  9:34         ` Daniel Vetter
2019-08-21  9:34           ` Daniel Vetter
2019-08-21 15:41       ` Daniel Vetter
2019-08-21 15:41         ` Daniel Vetter
2019-08-21 16:16         ` Jason Gunthorpe
2019-08-22  8:42           ` Daniel Vetter
2019-08-22  8:42             ` Daniel Vetter
2019-08-22 14:24             ` Jason Gunthorpe
2019-08-22 14:27               ` Daniel Vetter [this message]
2019-08-22 14:27                 ` Daniel Vetter
2019-08-20 11:15 ` ✗ Fi.CI.CHECKPATCH: warning for mmu notifier debug annotations/checks Patchwork
2019-08-20 12:33 ` ✓ Fi.CI.BAT: success " Patchwork
2019-08-20 18:14 ` ✗ Fi.CI.IGT: failure " Patchwork

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=CAKMK7uF5CMSUrG2jTYJ9M7tDK_Saxmxk6yLs62tfc-Ozj3p2BQ@mail.gmail.com \
    --to=daniel@ffwll.ch \
    --cc=akpm@linux-foundation.org \
    --cc=christian.koenig@amd.com \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jgg@ziepe.ca \
    --cc=jglisse@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=rientjes@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.