From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A3237217F27 for ; Fri, 29 May 2026 10:07:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.92.199 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780049225; cv=none; b=b6/qEiJrtY90cblMb+NQvV0n5kwJ+K5+bbU4MPkFAcwXM/mHdj2o5Ef3+VBv5lkdDU1SNa7pXrLmm+4uaJbp/H1Qr4AQnt6bgZgyG4ij78NVYPUmorDk3Y87LYykWf1h0XoNv5Prqc3KR1g9V4QPEu+e6KwxvuDfGRL9t4khLoQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780049225; c=relaxed/simple; bh=bJ1RjhK2qQU1LKjMDxYxwbdPKFXLum8pLEsFtnrffCw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=M3KeZGQnUJSqVBqcj4mefN/yMJ0hy2HUSdUBfT5TTlw750/OKnFPO6lM4zkWjXnalwcOEvW688VeTRTLPyyWLS1J5KM6GHCMELjQefCy7kqo1vB6k/J1tNJc8N/XihFJCRP8tXt9xDaV+XLcrnVX8i0jpuqxlh4Zh2+t42WbxDE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=PxN6NqPp; arc=none smtp.client-ip=90.155.92.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="PxN6NqPp" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=aI0QdMFbkvp1vPPaMC52uCcjjPYRT/qpPcpWgY11btk=; b=PxN6NqPp3eoyy1AL4Z3cErrMpL dPC69Ch6+AjIpCj6UUSMRPoLB2CRw8iN5sr+zsHTYEIGllfVlyOkUf0pVD+Pz41LCAACmpejcNwDA W4oF76mPYnMHrZrvKidaat+vGWJvtpZeui4mS+5lU3KfgtiAKy6JpLqUpaCx7otjRUgEc1SJ54TrF /6lzeFTjpiMUQIO8C2/LGKPWH92uqsiYxQX+IvLGL5a8myAjv0XqyacpiQyeXCDkn1Lmd2um0CygS Z822RDxsognsvRVsW5qyFBGJD9a2IRDhQGHYYWvf+afVklcJL4lthXbpj3rg8GT3B7k2FxydOuU6X 42jm9biA==; Received: from 77-249-17-252.cable.dynamic.v4.ziggo.nl ([77.249.17.252] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.99.1 #2 (Red Hat Linux)) id 1wSu79-00000000zsX-0H4A; Fri, 29 May 2026 10:06:51 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 112F2300673; Fri, 29 May 2026 12:06:50 +0200 (CEST) Date: Fri, 29 May 2026 12:06:49 +0200 From: Peter Zijlstra To: K Prateek Nayak Cc: John Stultz , Joel Fernandes , Qais Yousef , Ingo Molnar , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Valentin Schneider , Steven Rostedt , Ben Segall , Zimuzo Ezeozue , Will Deacon , Waiman Long , Boqun Feng , "Paul E. McKenney" , Metin Kaya , Xuewen Yan , Thomas Gleixner , Daniel Lezcano , Suleiman Souhlal , kuyo chang , hupu , linux-kernel@vger.kernel.org, Mike Galbraith Subject: Re: [PATCH 1/6] sched/proxy: Remove superfluous clear_task_blocked_in() Message-ID: <20260529100649.GB3144646@noisy.programming.kicks-ass.net> References: <20260526111609.433880331@infradead.org> <20260526113322.120970670@infradead.org> <9dd1d24d-45d3-4ee2-8e67-8305b34bfb6d@amd.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9dd1d24d-45d3-4ee2-8e67-8305b34bfb6d@amd.com> On Fri, May 29, 2026 at 01:28:45PM +0530, K Prateek Nayak wrote: > Hello John, > > On 5/29/2026 12:18 PM, John Stultz wrote: > > Bascially we can get in a situation where (sorry this gets a bit convoluted): > > > > 1) On CPU1, __mutex_lock_common, we set task A > > blocked_on/TASK_UNINTERRUPTABLE, and call into __schedule(). > > > > 2) On CPU2, task B who holds the mutex calls __mutex_unlock_slowpath() > > and sets task A as PROXY_WAKING and starts to call into > > wake_up_task(). > > > > 3) On CPU1, in __schedule() we pick_next_task(), which returns task A, > > which is_blocked. We call find_proxy_task() and note task A is > > PROXY_WAKING. Since its also current, we take the short-cut and clear > > is_blocked and blocked_on and return task A to run. > > > > 4) On CPU2, try_to_wake_up() hits ttwu_runnable(), and > > proxy_needs_return() returns false as A->blocked_on is zero. > > > > 5) On CPU 1, task A is running, it grabs the lock it was waiting for > > and exits __mutex_lock_common. It then enters __mutex_lock_common to > > grab a different mutex that is already locked. It sets itself > > blocked_on/TASK_INTERRUPTABLE and calls into __schedule() > > > > 6) On CPU2, ttwu_runnable() continues, and calls ttwu_do_wakeup(), > > which clears A->is_blocked and sets the A->__state TASK_RUNNING > > > > 7) On CPU3, task C that holds the mutex A is waiting on, calls > > __mutex_unlock_slowpath, setting A as PROXY_WAKING and calls into > > wake_up_task() > > > > 8) On CPU1, in __schedule() pick_next_task() again returns task A. But > > is_blocked is now zero, so we just return task A, even though > > blocked_on is PROXY_WAKING. > > > > 9) On CPU1, task A gets back to the __mutex_lock_common() loop, calls > > set_task_blocked_on() and trips warnings as A->blocked_on is still > > PROXY_WAKING. > > Oh geez! Me tries to visualize: Thanks!, I too need pictures, prose will forever confuse me :/ > > CPU1 CPU2 CPU3 > ==== ==== ==== > > __mutex_lock_common(MutexA) > set_task_blocked_on(TaskA, MutexA) > set_current_state(TASK_UNINTERRUPTABLE) __mutex_unlock_slowpath(MutexA) > ... set_task_blocked_on_waking(TaskA) > schedule_preempt_disabled() wake_up_process(TaskA) > __schedule() /* (1) */ ... /* (2) */ > > if (prev_state &..) > TaskA->is_blocked = 0; Should this be: TaskA->is_blocked = 1? Otherwise I'm not following. > next = TaskA > find_proxy_task(TaskA) > /* TaskA-> blocked_on == TASK_WAKING */ > clear_task_blocked_on(TaskA, NULL); > TaskA->is_blocked = 0; > ... > next = TaskA /* (3) */ > rq_unlock(CPU1) try_to_wake_up(TaskA) > rq_lock(CPU1) > ttwu_runnable() > /* TaskA->blocked_on == 0 (4) */ > ... > set_curent_state(TASK_RUNNING) > ... > > mutex_lock(MutexB) > __mutex_lock_common(MutexB) > ... > set_task_blocked_on(TaskA, MutexB) > set_current_state(TASK_UNINTERRUPTABLE) > schedule_preempt_disabled() > __schedule() /* (5) */ ... __mutex_unlock_slowpath(MutexB) > ttwu_do_wakeup(TaskA) /* (6) */ set_task_blocked_on_waking(TaskA) /* (7) */ > rq_unlock(CPU1) > > rq_lock(CPU1) > /* TaskA->__state == TASK_RUNNING */ > next = TasKA; > > if (TaskA->is_blocked /* False */ && TaskA->blocked_on /* PROXY_WAKING */) > /* Skip */ > next = TaskA; /* (8) */ > > set_task_blocked_on(p, MutexB) > > !!! p->blocked_on != MutexB !!! > > > Yup! That is a concern too then! > > I think we can just squash the PROXY_WAKING removal with "p->is_blocked" > introduction and a part of this problem should go away since unlocks > always clear task->blocked_on then. While staring at this, I noted that the PROXY_WAKING removal patch should also remove the clear_task_blocked_on() line in the very last hunk. That said, I do have a note to double check the lockless access to p->blocked_on there. Anyway, yes, the hunk removed in patch 1 cures this by clearing TaskA->blocked_on (because prev == current == TaskA). I don't think we need to squash everything into one giant patch over this if we just reorder things. The Changelog of patch 1 needs an extra few links to this discussion, but that should be it, no?