Re: [RFC][PATCH v3 4/4] printk: set may_schedule for some of console_trylock callers

From: Petr Mladek <pmladek@suse.com>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jan Kara <jack@suse.com>, Tejun Heo <tj@kernel.org>,
	Kyle McMartin <kyle@kernel.org>,
	Dave Jones <davej@codemonkey.org.uk>,
	Calvin Owens <calvinowens@fb.com>,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: Re: [RFC][PATCH v3 4/4] printk: set may_schedule for some of console_trylock callers
Date: Thu, 11 Feb 2016 17:10:00 +0100	[thread overview]
Message-ID: <20160211161000.GN3305@pathway.suse.cz> (raw)
In-Reply-To: <20160211150217.GA527@swordfish>

On Fri 2016-02-12 00:02:17, Sergey Senozhatsky wrote:
> Hello Petr,
> 
> On (02/11/16 15:41), Petr Mladek wrote:
> [..]
> > > +	console_may_schedule = !oops_in_progress &&
> > > +			preemptible() &&
> > > +			!rcu_preempt_depth();
> > >  	return 1;
> > 
> > We discussed this a lot but I am still a bit nervous ;-)
> 
> sure, no prob :-)
> 
> > Avoid scheduling when oops_in_progress makes sense.
> > 
> > preemptible() takes care of preemption and IRQ contexts.
> > The comment above explains that it is safe to use here.
> > 
> > The check for rcu_preempt_depth() makes sense. But is it
> > safe, please?
> > 
> > rcu_preempt_depth() returns 0 if CONFIG_PREEMPT_RCU is not
> > enabled. It means that you are not able to detect RCU read
> > section and it might cause problems.
> 
> well, I believe it's ok. __rcu_read_lock() for CONFIG_PREEMPT_RCU
> does current->rcu_read_lock_nesting++, so rcu_preempt_depth() works
> as expected. otherwise, for !CONFIG_PREEMPT_RCU kernel,
> __rcu_read_lock() does
> 
> 	if (IS_ENABLED(CONFIG_PREEMPT_COUNT))
> 		preempt_disable()
> 
> 
> - if we run "CONFIG_PREEMPT_RCU" then rcu_preempt_depth()
>   works here.
> 
> - if we run "!CONFIG_PREEMPT_RCU && CONFIG_PREEMPT_COUNT"
>   then preemptible() works for us
> 
> - if we run "!CONFIG_PREEMPT_RCU && !CONFIG_PREEMPT_COUNT"
>   then preemptible() is always 0.

I feel convinced. But we should somehow document it. I think how
to do it effectively. I think that the following text would help
me if I read it:

	/*
	 * Safe context for rescheduling is detected only when
	 * PREEMPT_COUNT is enabled. preemptible() always returns
	 * false otherwise.
	 *
	 * RCU read sections must be detected separately. They
	 * have a separate preemption counter when PREEMPT_RCU
	 * is enabled.
	 */

I wanted to highlight why exactly the check returns 0 in !PREEMPT_COUNT
kernel. I missed this a bit in you original comment. But feel free
to change it as you like.

Best Regards,
Petr