All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: "Nicolai Hähnle" <nhaehnle@gmail.com>,
	"Nicolai Hähnle" <Nicolai.Haehnle@amd.com>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	stable <stable@vger.kernel.org>, "Ingo Molnar" <mingo@redhat.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"Maarten Lankhorst" <maarten.lankhorst@canonical.com>
Subject: Re: [PATCH 1/4] locking/ww_mutex: Fix a deadlock affecting ww_mutexes
Date: Thu, 24 Nov 2016 12:56:01 +0100	[thread overview]
Message-ID: <20161124115601.GR3207@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <CAKMK7uEBMg8Hps5rW6hK4sgzTr9ek2jvG35HzePp1TWw-uA4HA@mail.gmail.com>

On Thu, Nov 24, 2016 at 12:52:25PM +0100, Daniel Vetter wrote:
> On Thu, Nov 24, 2016 at 12:40 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> >> I do believe we can win a bit by keeping the wait list sorted, if we also
> >> make sure that waiters don't add themselves in the first place if they see
> >> that a deadlock situation cannot be avoided.
> >>
> >> I will probably want to extend struct mutex_waiter with ww_mutex-specific
> >> fields to facilitate this (i.e. ctx pointer, perhaps stamp as well to reduce
> >> pointer-chasing). That should be fine since it lives on the stack.
> >
> > Right, shouldn't be a problem I think.
> >
> > The only 'problem' I can see with using that is that its possible to mix
> > ww and !ww waiters through ww_mutex_lock(.ctx = NULL). This makes the
> > list order somewhat tricky.
> >
> > Ideally we'd remove that feature, although I see its actually used quite
> > a bit :/
> 
> I guess we could create a small fake acquire_ctx for single-lock
> paths. That way callers still don't need to deal with having an
> explicit ctx, but we can assume the timestamp (for ensuring fairness)
> is available for all cases. Otherwise there's indeed a problem with
> correctly (well fairly) interleaving ctx and non-ctx lockers I think.

Actually tried that, but we need a ww_class to get a stamp from, and
ww_mutex_lock() doesn't have one of those..

WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: "Nicolai Hähnle" <Nicolai.Haehnle@amd.com>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"Ingo Molnar" <mingo@redhat.com>, stable <stable@vger.kernel.org>,
	"Maarten Lankhorst" <maarten.lankhorst@canonical.com>
Subject: Re: [PATCH 1/4] locking/ww_mutex: Fix a deadlock affecting ww_mutexes
Date: Thu, 24 Nov 2016 12:56:01 +0100	[thread overview]
Message-ID: <20161124115601.GR3207@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <CAKMK7uEBMg8Hps5rW6hK4sgzTr9ek2jvG35HzePp1TWw-uA4HA@mail.gmail.com>

On Thu, Nov 24, 2016 at 12:52:25PM +0100, Daniel Vetter wrote:
> On Thu, Nov 24, 2016 at 12:40 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> >> I do believe we can win a bit by keeping the wait list sorted, if we also
> >> make sure that waiters don't add themselves in the first place if they see
> >> that a deadlock situation cannot be avoided.
> >>
> >> I will probably want to extend struct mutex_waiter with ww_mutex-specific
> >> fields to facilitate this (i.e. ctx pointer, perhaps stamp as well to reduce
> >> pointer-chasing). That should be fine since it lives on the stack.
> >
> > Right, shouldn't be a problem I think.
> >
> > The only 'problem' I can see with using that is that its possible to mix
> > ww and !ww waiters through ww_mutex_lock(.ctx = NULL). This makes the
> > list order somewhat tricky.
> >
> > Ideally we'd remove that feature, although I see its actually used quite
> > a bit :/
> 
> I guess we could create a small fake acquire_ctx for single-lock
> paths. That way callers still don't need to deal with having an
> explicit ctx, but we can assume the timestamp (for ensuring fairness)
> is available for all cases. Otherwise there's indeed a problem with
> correctly (well fairly) interleaving ctx and non-ctx lockers I think.

Actually tried that, but we need a ww_class to get a stamp from, and
ww_mutex_lock() doesn't have one of those..
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2016-11-24 12:19 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-23 11:25 [PATCH 1/4] locking/ww_mutex: Fix a deadlock affecting ww_mutexes Nicolai Hähnle
2016-11-23 11:25 ` [PATCH 2/4] locking/ww_mutex: Remove redundant wakeups in ww_mutex_set_context_slowpath Nicolai Hähnle
2016-11-23 11:25   ` Nicolai Hähnle
2016-11-23 11:25 ` [PATCH 3/4] locking/Documentation: fix a typo Nicolai Hähnle
2016-11-23 11:25   ` Nicolai Hähnle
2016-11-23 11:25 ` [PATCH 4/4] locking/ww_mutex: Fix a comment typo Nicolai Hähnle
2016-11-23 11:25   ` Nicolai Hähnle
2016-11-23 12:50 ` [PATCH 1/4] locking/ww_mutex: Fix a deadlock affecting ww_mutexes Daniel Vetter
2016-11-23 12:50   ` Daniel Vetter
2016-11-23 13:00 ` Peter Zijlstra
2016-11-23 13:00   ` Peter Zijlstra
2016-11-23 13:00   ` Peter Zijlstra
2016-11-23 13:08   ` Daniel Vetter
2016-11-23 13:08     ` Daniel Vetter
2016-11-23 13:11     ` Daniel Vetter
2016-11-23 13:11       ` Daniel Vetter
2016-11-23 13:11       ` Daniel Vetter
2016-11-23 13:33       ` Maarten Lankhorst
2016-11-23 13:33         ` Maarten Lankhorst
2016-11-23 14:03 ` Peter Zijlstra
2016-11-23 14:03   ` Peter Zijlstra
2016-11-23 14:03   ` Peter Zijlstra
2016-11-23 14:25   ` Daniel Vetter
2016-11-23 14:25     ` Daniel Vetter
2016-11-23 14:25     ` Daniel Vetter
2016-11-23 14:32     ` Peter Zijlstra
2016-11-23 14:32       ` Peter Zijlstra
2016-11-24 11:26     ` Nicolai Hähnle
2016-11-24 11:26       ` Nicolai Hähnle
2016-11-24 11:40       ` Peter Zijlstra
2016-11-24 11:40         ` Peter Zijlstra
2016-11-24 11:40         ` Peter Zijlstra
2016-11-24 11:52         ` Daniel Vetter
2016-11-24 11:52           ` Daniel Vetter
2016-11-24 11:56           ` Peter Zijlstra [this message]
2016-11-24 11:56             ` Peter Zijlstra
2016-11-24 12:05             ` Nicolai Hähnle

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=20161124115601.GR3207@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=Nicolai.Haehnle@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@canonical.com \
    --cc=mingo@redhat.com \
    --cc=nhaehnle@gmail.com \
    --cc=stable@vger.kernel.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 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.