From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758133AbcGKM1L (ORCPT ); Mon, 11 Jul 2016 08:27:11 -0400 Received: from forward-corp1f.mail.yandex.net ([95.108.130.40]:38029 "EHLO forward-corp1f.mail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753897AbcGKM1J (ORCPT ); Mon, 11 Jul 2016 08:27:09 -0400 Authentication-Results: smtpcorp1m.mail.yandex.net; dkim=pass header.i=@yandex-team.ru Subject: Re: [PATCH] sched/fair: do not announce throttled next buddy in dequeue_task_fair To: xlpang@redhat.com, Wanpeng Li References: <146608183552.21905.15924473394414832071.stgit@buzz> <57835735.6020906@redhat.com> <57835BFD.90201@redhat.com> <57838D28.4090003@redhat.com> Cc: Peter Zijlstra , Ingo Molnar , "linux-kernel@vger.kernel.org" , stable@vger.kernel.org From: Konstantin Khlebnikov Message-ID: <5783907A.6030609@yandex-team.ru> Date: Mon, 11 Jul 2016 15:26:34 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <57838D28.4090003@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11.07.2016 15:12, Xunlei Pang wrote: > On 2016/07/11 at 17:54, Wanpeng Li wrote: >> Hi Konstantin, Xunlei, >> 2016-07-11 16:42 GMT+08:00 Xunlei Pang : >>> On 2016/07/11 at 16:22, Xunlei Pang wrote: >>>> On 2016/07/11 at 15:25, Wanpeng Li wrote: >>>>> 2016-06-16 20:57 GMT+08:00 Konstantin Khlebnikov : >>>>>> Hierarchy could be already throttled at this point. Throttled next >>>>>> buddy could trigger null pointer dereference in pick_next_task_fair(). >>>>> There is cfs_rq->next check in pick_next_entity(), so how can null >>>>> pointer dereference happen? >>>> I guess it's the following code leading to a NULL se returned: >>> s/NULL/empty-entity cfs_rq se/ >>> >>>> pick_next_entity(): >>>> if (cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, left) < 1) >> ^^^^^^^^^^^^^ >> I think this will return false. > > With the wrong throttled_hierarchy(), I think this can happen. But after we have the > corrected throttled_hierarchy() patch, I can't see how it is possible. > > dequeue_task_fair(): > if (task_sleep && parent_entity(se)) > set_next_buddy(parent_entity(se)); > > How does dequeue_task_fair() with DEQUEUE_SLEEP set(true task_sleep) happen to a throttled hierarchy? > IOW, a task belongs to a throttled hierarchy is running? > > Maybe Konstantin knows the reason. This function (dequeue_task_fair) check throttling but at point it could skip several levels and announce as next buddy actually throttled entry. Probably this bug hadn't happened but this's really hard to prove that this is impossible. ->set_curr_task(), PI-boost or some tricky migration in balancer could break this easily. -- Konstantin