LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: joel@joelfernandes.org, rcu@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-team@fb.com,
	mingo@kernel.org, jiangshanlai@gmail.com, dipankar@in.ibm.com,
	akpm@linux-foundation.org, mathieu.desnoyers@efficios.com,
	josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org,
	rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com,
	fweisbec@gmail.com, oleg@redhat.com,
	Uladzislau Rezki <urezki@gmail.com>
Subject: Re: [PATCH tip/core/rcu 03/17] rcu/tree: Skip entry into the page allocator for PREEMPT_RT
Date: Tue, 30 Jun 2020 11:35:34 -0700
Message-ID: <20200630183534.GG9247@paulmck-ThinkPad-P72> (raw)
In-Reply-To: <20200630164543.4mdcf6zb4zfclhln@linutronix.de>

On Tue, Jun 30, 2020 at 06:45:43PM +0200, Sebastian Andrzej Siewior wrote:
> On 2020-06-24 13:12:12 [-0700], paulmck@kernel.org wrote:
> > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> > 
> > To keep the kfree_rcu() code working in purely atomic sections on RT,
> > such as non-threaded IRQ handlers and raw spinlock sections, avoid
> > calling into the page allocator which uses sleeping locks on RT.
> > 
> > In fact, even if the  caller is preemptible, the kfree_rcu() code is
> > not, as the krcp->lock is a raw spinlock.
> > 
> > Calling into the page allocator is optional and avoiding it should be
> > Ok, especially with the page pre-allocation support in future patches.
> > Such pre-allocation would further avoid the a need for a dynamically
> > allocated page in the first place.
> > 
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Reviewed-by: Uladzislau Rezki <urezki@gmail.com>
> > Co-developed-by: Uladzislau Rezki <urezki@gmail.com>
> > Signed-off-by: Uladzislau Rezki <urezki@gmail.com>
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > ---
> >  kernel/rcu/tree.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 64592b4..dbdd509 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -3184,6 +3184,18 @@ kfree_call_rcu_add_ptr_to_bulk(struct kfree_rcu_cpu *krcp,
> >  		if (!bnode) {
> >  			WARN_ON_ONCE(sizeof(struct kfree_rcu_bulk_data) > PAGE_SIZE);
> >  
> > +			/*
> > +			 * To keep this path working on raw non-preemptible
> > +			 * sections, prevent the optional entry into the
> > +			 * allocator as it uses sleeping locks. In fact, even
> > +			 * if the caller of kfree_rcu() is preemptible, this
> > +			 * path still is not, as krcp->lock is a raw spinlock.
> > +			 * With additional page pre-allocation in the works,
> > +			 * hitting this return is going to be much less likely.
> > +			 */
> > +			if (IS_ENABLED(CONFIG_PREEMPT_RT))
> > +				return false;
> 
> This is not going to work together with the "wait context validator"
> (CONFIG_PROVE_RAW_LOCK_NESTING). As of -rc3 it should complain about
> printk() which is why it is still disabled by default.

Fixing that should be "interesting".  In particular, RCU CPU stall
warnings rely on the raw spin lock to reduce false positives due
to race conditions.  Some thought will be required here.

> So assume that this is fixed and enabled then on !PREEMPT_RT it will
> complain that you have a raw_spinlock_t acquired (the one from patch
> 02/17) and attempt to acquire a spinlock_t in the memory allocator.

Given that the slab allocator doesn't acquire any locks until it gets
a fair way in, wouldn't it make sense to allow a "shallow" allocation
while a raw spinlock is held?  This would require yet another GFP_ flag,
but that won't make all that much of a difference in the total.  ;-)

							Thanx, Paul

> >  			bnode = (struct kfree_rcu_bulk_data *)
> >  				__get_free_page(GFP_NOWAIT | __GFP_NOWARN);
> >  		}
> 
> Sebastian

  reply index

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-24 20:12 [PATCH tip/core/rcu 0/17] kfree_rcu updates for v5.9 Paul E. McKenney
2020-06-24 20:12 ` [PATCH tip/core/rcu 01/17] rcu: Fix a kernel-doc warnings for "count" paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 02/17] rcu/tree: Keep kfree_rcu() awake during lock contention paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 03/17] rcu/tree: Skip entry into the page allocator for PREEMPT_RT paulmck
2020-06-30 16:45   ` Sebastian Andrzej Siewior
2020-06-30 18:35     ` Paul E. McKenney [this message]
2020-07-02 14:12       ` Sebastian Andrzej Siewior
2020-07-02 16:48         ` Paul E. McKenney
2020-07-02 20:19           ` Sebastian Andrzej Siewior
2020-07-06 21:06             ` Paul E. McKenney
2020-07-07 17:34               ` Uladzislau Rezki
2020-07-07 18:45                 ` Joel Fernandes
2020-07-08 18:48                   ` Uladzislau Rezki
2020-07-02 19:45         ` Uladzislau Rezki
2020-07-06 19:42           ` Joel Fernandes
2020-07-06 19:55             ` Uladzislau Rezki
2020-07-06 20:29               ` Joel Fernandes
2020-07-07  9:27                 ` Sebastian Andrzej Siewior
2020-07-15 13:38     ` Uladzislau Rezki
2020-07-15 14:16       ` Sebastian Andrzej Siewior
2020-07-15 14:20         ` Uladzislau Rezki
2020-06-24 20:12 ` [PATCH tip/core/rcu 04/17] rcu/tree: Repeat the monitor if any free channel is busy paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 05/17] rcu/tree: Make debug_objects logic independent of rcu_head paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 06/17] rcu/tree: Simplify KFREE_BULK_MAX_ENTR macro paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 07/17] rcu/tree: Move kfree_rcu_cpu locking/unlocking to separate functions paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 08/17] rcu/tree: Use static initializer for krc.lock paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 09/17] rcu/tree: cache specified number of objects paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 10/17] rcu/tree: Maintain separate array for vmalloc ptrs paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 11/17] rcu/tiny: support vmalloc in tiny-RCU paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 12/17] rcu: Rename *_kfree_callback/*_kfree_rcu_offset/kfree_call_* paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 13/17] mm/list_lru.c: Rename kvfree_rcu() to local variant paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 14/17] rcu: Introduce 2 arg kvfree_rcu() interface paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 15/17] rcu: Support reclaim for head-less object paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 16/17] rcu: Introduce single argument kvfree_rcu() interface paulmck
2020-06-24 20:12 ` [PATCH tip/core/rcu 17/17] lib/test_vmalloc.c: Add test cases for kvfree_rcu() paulmck

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=20200630183534.GG9247@paulmck-ThinkPad-P72 \
    --to=paulmck@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bigeasy@linutronix.de \
    --cc=dhowells@redhat.com \
    --cc=dipankar@in.ibm.com \
    --cc=edumazet@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=urezki@gmail.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git