All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Will Deacon <will@kernel.org>, Waiman Long <longman@redhat.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH] locking: Remove rt_rwlock_is_contended()
Date: Mon, 13 Sep 2021 13:20:32 +0200	[thread overview]
Message-ID: <YT80AB8/G59QBSVq@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20210910163704.ykotcrvbt6yaqron@linutronix.de>

On Fri, Sep 10, 2021 at 06:37:04PM +0200, Sebastian Andrzej Siewior wrote:
> On 2021-09-10 18:16:14 [+0200], Peter Zijlstra wrote:
> > On Tue, Sep 07, 2021 at 12:34:58PM +0200, Sebastian Andrzej Siewior wrote:

> Yes. I got arguments against it after sleeping :)

Sleep is magical :-)

> > AFAICT the _is_contended() can still use useful even with preemption,
> > the typicla use case is a long lock-holder deciding to drop the lock in
> > order to let someone else in. That still works with preemptible locks,
> > no?
> 
> Sure. We can do that. Then we should look into:
> - fixing rwsem_is_contended() for the writer. The writer always observes
>   true even with no waiter around.

Right, that function does look somewhat dodgy. I'm thinking the current
function returns true if there's more than a single reader present (or a
writer) present, which is not the same.

I suppose it shoud return something like:

  for a writer: rt_mutex_is_contended(&rwb->rtmutex);
  for a reader: rt_mutex_is_locked(&rwb->rtmutex);

However, given the below arguments,,,

> - checking the top waiter list vs priority of the lock owner/current. If
>   the current lock owner has the highest priority then the unlock+lock
>   is probably pointless as he regains the lock.
>   For the spin_lock() case, if the owner is SCHED_OTHER and the waiter
>   is SCHED_OTHER then unlock+lock will give the lock to the previous
>   owner due to rt_mutex_steal() working in his favour. Unless there is a
>   preemption.

That is a good argument against all this; I had not considered that.

> - reader checking for contention is probably pointless. It works with a
>   pending writer and one reader since a second reader will hold-off the
>   writer from acquiring the lock. Also if the reader does unlock+lock
>   then writer might not be quick enough.

Should be fixable with a handoff, but yeah.

OK, I suppose the safe and easy option is to never report contention as
per your latest patch, and if/when someone complains about it, they can
sort through these issues :-)

      reply	other threads:[~2021-09-13 11:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-06 14:30 [PATCH 0/2] locking: Provide {rwlock|spin}_is_contended on PREEMPT_RT Sebastian Andrzej Siewior
2021-09-06 14:30 ` [PATCH 1/2] locking: Wire up rwlock_is_contended() " Sebastian Andrzej Siewior
2021-09-06 14:30 ` [PATCH 2/2] locking: Provide spin_is_contended() " Sebastian Andrzej Siewior
2021-09-07 10:09 ` [PATCH 0/2] locking: Provide {rwlock|spin}_is_contended " Sebastian Andrzej Siewior
2021-09-07 10:34   ` [PATCH] locking: Remove rt_rwlock_is_contended() Sebastian Andrzej Siewior
2021-09-10 16:16     ` Peter Zijlstra
2021-09-10 16:37       ` Sebastian Andrzej Siewior
2021-09-13 11:20         ` Peter Zijlstra [this message]

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=YT80AB8/G59QBSVq@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bigeasy@linutronix.de \
    --cc=boqun.feng@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=will@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.