All of lore.kernel.org
 help / color / mirror / Atom feed
From: Byungchul Park <byungchul.park@lge.com>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: rcu <rcu@vger.kernel.org>, kernel-team@lge.com
Subject: Re: This percpu_rwsem that always enters its reader slow path
Date: Fri, 19 Jul 2019 16:50:11 +0900	[thread overview]
Message-ID: <20190719075011.GD28226@X58A-UD3R> (raw)
In-Reply-To: <CAEXW_YR39bnCCj+eo30jSZrNEvcJY8wrXuA0fYr_gfbfNiVsPQ@mail.gmail.com>

On Thu, Jul 18, 2019 at 05:09:45PM -0400, Joel Fernandes wrote:
> Hello friends,
> 
> Just providing an update on my debugging of percpu_rwsem (related to
> rcu-sync) for the day! which I pinged Byungchul about. Please ignore
> this email if you are busy :) I am just archiving it in here..
> 
> As you may know, percpu_rwsem uses rcu-sync framework to reduce cost
> of read-side by making it free of any serializing/atomic instructions
> at all. However, there was one sempahore which broke the rules!
> 
> I spent a couple hours trying to figure out why
> cgroup_threadgroup_rwsem always entered the reader-slow path on my
> system (RCU-sync turns out to be non-idle for this rwsem). I really
> thought it was a bug, because I felt what's the pointed of rcu-sync if
> it never goes idle..

Yes, with the following patch, the cgroup rwsem cannot make use of
rcu_sync any more, but it still gets benefit from percpu structure
as you told me like avoiding cache bouncing and contention on a shared
area even though every read lock keeps firing smp full barrier.

What matters is which one is more expensive between (1) firing smp_mb
and (2) accessing a shared data, sem->count, and acquiring/releasing
sem->wait_lock. I think using percpu-rwsem involving the smp barrier is
much better even with rcu_sync disabled.

Or am I missing the point? Please let me know if so.

Thanks,
Byungchul

> Then I landed on the commit below, and turns it was done for Android
> and reported by John :) And the patch author was a certain guy named
> Peter :)
> 
> commit 3942a9bd7b5842a924e99ee6ec1350b8006c94ec
> Author: Peter Zijlstra <peterz@infradead.org>
> Date:   Thu Aug 11 18:54:13 2016 +0200
> 
>     locking, rcu, cgroup: Avoid synchronize_sched() in __cgroup_procs_write()
> -----------
> 
> Basically, this commit makes the read-side cost percpu_rwsem slightly
> more expensive (one smp_load_acquire of readers_block, at the cost of
> making write-side a bit more expensive...)
> 
> So turns out it is weird, but it is certainly not a bug.
> 
> Learned something new but wasted my time a bit :)
> 
> Cheers, and see you later,
> - Joel

  parent reply	other threads:[~2019-07-19  7:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-18 21:09 This percpu_rwsem that always enters its reader slow path Joel Fernandes
2019-07-18 21:10 ` Joel Fernandes
2019-07-19  7:50 ` Byungchul Park [this message]
2019-07-21 13:28   ` Joel Fernandes

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=20190719075011.GD28226@X58A-UD3R \
    --to=byungchul.park@lge.com \
    --cc=joel@joelfernandes.org \
    --cc=kernel-team@lge.com \
    --cc=rcu@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.