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;
Subject: Re: [BUG] kernel: rcu: a possible sleep-in-atomic-context bug in
 srcu_read_delay()
To:     paulmck@linux.vnet.ibm.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>
References: <4179b4d2-b832-abcd-d407-b865765f28d6@gmail.com>
 <20180813041826.GL24813@linux.vnet.ibm.com>
 <5b3fd721-541c-5165-ef54-d6c476318d92@gmail.com>
 <20180813124232.GM24813@linux.vnet.ibm.com>
From:   Jia-Ju Bai <baijiaju1990@gmail.com>
Message-ID: <5a9b96db-07a2-67bb-c2ab-6ee1f5c0e24d@gmail.com>
Date:   Wed, 15 Aug 2018 09:05:16 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.0
MIME-Version: 1.0
In-Reply-To: <20180813124232.GM24813@linux.vnet.ibm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2018/8/13 20:42, Paul E. McKenney wrote:
> On Mon, Aug 13, 2018 at 05:26:49PM +0800, Jia-Ju Bai wrote:
>>
>> On 2018/8/13 12:18, Paul E. McKenney wrote:
>>> 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.
>> Thanks for your reply :)
>> My tool does not track this bit...
>> Sorry for this false report.
> Not a problem, a few false positives are to be expected.  And it looks
> like you have some work to do on your tool -- which is good, because I
> would not want you to be bored.  ;-)
>
> 							Thanx, Paul
>

Thanks for your advice.
I will improve my tool to produce less false positives :)


Best wishes,
Jia-Ju Bai