Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sun, 12 Aug 2018 21:18:26 -0700
From:   "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To:     Jia-Ju Bai <baijiaju1990@gmail.com>
Cc:     dave@stgolabs.net, josh@joshtriplett.org, rostedt@goodmis.org,
        mathieu.desnoyers@efficios.com, jiangshanlai@gmail.com,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [BUG] kernel: rcu: a possible sleep-in-atomic-context bug in
 srcu_read_delay()
Reply-To: paulmck@linux.vnet.ibm.com
References: <4179b4d2-b832-abcd-d407-b865765f28d6@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4179b4d2-b832-abcd-d407-b865765f28d6@gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Message-Id: <20180813041826.GL24813@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Aug 13, 2018 at 11:04:10AM +0800, Jia-Ju Bai wrote:
> The kernel may sleep with holding a spinlock.
> 
> The function call paths (from bottom to top) in Linux-4.16 are:
> 
> [FUNC] schedule_timeout_interruptible
> kernel/rcu/rcutorture.c, 523: schedule_timeout_interruptible in
> srcu_read_delay
> kernel/rcu/rcutorture.c, 1105: [FUNC_PTR]srcu_read_delay in
> rcu_torture_timer
> kernel/rcu/rcutorture.c, 1104: spin_lock in rcu_torture_timer
> 
> Note that [FUNC_PTR] means a function pointer call is used.
> 
> I do not find a good way to fix, so I only report.
> This is found by my static analysis tool (DSAC).

Interesting.  I would have expected to have gotten a "scheduling while
atomic" error message, which I do not recall seeing.  And I ran a great
deal of rcutorture on v4.16.

So let's see...  As you say, the rcu_torture_timer() function does in
fact acquire rand_lock in 4.16 and 4.17, in which case sleeping would
indeed be illegal.  But let's take a look at srcu_read_delay():

static void
srcu_read_delay(struct torture_random_state *rrsp, struct rt_read_seg *rtrsp)
{
	long delay;
	const long uspertick = 1000000 / HZ;
	const long longdelay = 10;

	/* We want there to be long-running readers, but not all the time. */

	delay = torture_random(rrsp) %
		(nrealreaders * 2 * longdelay * uspertick);
	if (!delay && in_task()) {
		schedule_timeout_interruptible(longdelay);
		rtrsp->rt_delay_jiffies = longdelay;
	} else {
		rcu_read_delay(rrsp, rtrsp);
	}
}

The call to schedule_timeout_interruptible() cannot happen unless the
in_task() macro returns true, which it won't if the SOFTIRQ_OFFSET bit
is set:

#define in_task()		(!(preempt_count() & \
				   (NMI_MASK | HARDIRQ_MASK | SOFTIRQ_OFFSET)))

And the SOFTIRQ_OFFSET bit will be set if srcu_read_delay()
is invoked from a timer handler, which is the case for the
call from rcu_torture_timer().  So if that lock is held,
schedule_timeout_interruptible() won't ever be invoked.

So what am I missing here?

							Thanx, Paul