From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6D5873168EE for ; Thu, 2 Apr 2026 18:32:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=209.85.208.52 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775154734; cv=pass; b=hXTLj7LatTi+QnTkGMLoZ6nFqBfzHCtkmAGQ5UIAGjvhQutw033FKJx0isDGra/Ff58vdJBUMsxtat39ycMUpQLhJ8QX6EMHRTRfpemtxQNQgKyEI5uQF8rvdnRteb2049YuyCqaDuFqORgbGoia+XeVPjEDUoEc2OuGNSPK2mw= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775154734; c=relaxed/simple; bh=j2OGQQ1mIxhHQiqGcJZlZ/Jh8KQX5hpy20ejnbsP3qU=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=lahJKkhAxnB6AFTdLUk6VS4z1h7FCMbF/rO2lzrcZ4+Ksjki/75ZapLYTR2qQ9/oWXYyaBQyuK0Q9DJTwOUQgFLu32jlVOCFtMPQtCiJeQEsiLE0D43RfKbXKv0AmN9SatQ1Htu6QxSmnPhivf/OLJEGFn8lBPRj8rSwZAGIQy4= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=rQESuinJ; arc=pass smtp.client-ip=209.85.208.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="rQESuinJ" Received: by mail-ed1-f52.google.com with SMTP id 4fb4d7f45d1cf-66ba9898ae8so3879510a12.1 for ; Thu, 02 Apr 2026 11:32:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1775154731; cv=none; d=google.com; s=arc-20240605; b=GrRhXWp+S5g45K54XD937zhNwlqgo5czj3wt2U72QQq0RZ1DQfLJSnVaBDsd9AOwJd VOnt8GN77MPXUomFUM1Uzr/irdS0itLEaH94G2ZbyE0h/kI3PG2h0975oZ14Z8KNEQtJ qu+UOdK07VyjkcP2Shf8bKBSJmbKpUE+o/6QaK9hhXZ5JuR9vXuD52MuS7nG40ZPf/iW TfTYrb5hzck4+KP5zZvtJgoP+GRd99RxSgnYDnLiFZP4mngcjrzO3cVO5LtSVSCpfBmO RkLmeWCEJJvSBSdfZro67iWX6ppIgeoI3Fiqdkh5Q2FQfo5EIJyhQlpZ30wGaZ8ToKkH w03g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=GNiGpunSLZW/FwzoRqr8Z18/9DwWWoLgRoFfEO9PWKI=; fh=4DeyJKSWvSPfZBf0bKHqm/cIvYeLTk7ine2HskvREek=; b=CzLK8ZrVCCX4nOesjbeHxh0H8nm8Ac4rmVaqulaKzDRHMWEB94X62u6T8bc2Syh3Ut i7B20E4q4pwjvUnHI5DHoEfrC9+a+ec23Z9sPZHgpmW3tCUfGU/g+HJHFbEE6kEYKqFT QPrC4lNGxewvhehHWo57VKrH3KXhOkK4CuyhNRYM7A/+ZSEkZZu1Is7qtZvnM+ouD7tR hudTBL1e7M0TSkPmM2dU19DF824OaEfgQcR4SU5KFDTNibBLGsuifhdh+gFdKdjsLIv/ uKoJbzrY8ko/H/lLwtJs0ghZouUWrHbDhlDMBJEHvFcEG2HEYN5aPH5hjVYQRvDTan4x Y0OA==; darn=vger.kernel.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1775154731; x=1775759531; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=GNiGpunSLZW/FwzoRqr8Z18/9DwWWoLgRoFfEO9PWKI=; b=rQESuinJuVI3S+tRW0dAAZ7ZytWNP7UpPn9N9fx8JLCvsKBsVy6WsnwAMokLZarGTO J37d6Vyy1heSYg6u06upWUt5HrX8nhQvF/FKPEtopFYhguDSJr8vYYrn9Xc3XwYBHuXk xFLR/060sr44ovn+Au0iB8IS1sq072c+GxXuGQwhQG16NvwQANr3DzWI7l2TeP1Ya1fy DOF3K3yPS+9uzX1Q1BipD2Bh2JyvkQ1dwMhvAQXVEu2bSrq3EVpdYy6FbtNl5nrrz4LV 3fpwA4GMrURQyHZYEYac54N2RIIEyYuv/Msi3vlR4teeQMlJZ4qvPNFLgYhaYIIqEheC 6utQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775154731; x=1775759531; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=GNiGpunSLZW/FwzoRqr8Z18/9DwWWoLgRoFfEO9PWKI=; b=F2WYkQ11C0cP8rO7ge0RQ2HYzWPEsHZCzh0nrKZANpH0S+zZVSlkPE4IBVuZu+BUIg y/H0tE65lDVIA48c4vgVbkBhr4O6h8AuPIDffJ4rtNIA9reM7Q+/DotFwuxJ5PVYZ/+q Jz2KIzZyNaDsAzVgZzR4ZA/wvEK9uaHmzBLnaXXTweJMoq2kfguFJh9QpwEV4mX3uH2W hRL6UkbWTXtKACo9aqXx3xRDzTbi/1RbW4hAhAH1PLRMnauv1ArnoTIZ2VeMFEyDb3BK a3Dn/rvXpz2R7BeB7ThpdMshBANKX+PN1AzRfqKKP3xCpGy7kGBc74u07pBkKDzesFuD 1YIA== X-Forwarded-Encrypted: i=1; AJvYcCUcSrL0s6jz++ucrw4AQQ5ZepGreOvlkPxKXhd0rs5fAmJPrAG8T7KRAsm2L5ysxJ96E93qBawtxt8B+f0=@vger.kernel.org X-Gm-Message-State: AOJu0YworbJ7Dv9kutHIey8PU075UqImCfdEghmB2OLXbvQNRCntqpZi odHD6xhvYJ0lcR5Rwn2lVq20qdolHqU40IgsDwLEZfUXQ/r3if5KUdFINhTh1DgSWJLsJVk9h+L 84PRQXr1PHHS4Cgj78Bphb0HEpxwCRwuOQ9NPrXcIMF36kLFIW7hSC3Q= X-Gm-Gg: ATEYQzxgh3YgrU3Uq6q7MhGpEgSAZE3MCEQGazC/2bJh+hp9vNScQXUqULkCP0P9BkU /b87h6xRvoT0nUaV/BswCIWkc4kn1FS93oS0uv3ANVFJLu2DNFPDBwgGO8A7YiVUHwhd1MAbqZB xKHZAGSA6UCA06DkYHRWlREgwgCxbj1nx32E5SbrH4HDRGH8YQ1/g5E2yrNEkccVS3C4eDF0823 DSs4BLkXSWqQq8Fe/giXxbEJqvq+/MTBmh21rY3actChPpMtlWb9g536maklA7SqOPvc/G/ZGLi uTkbnBa2l3/MPLerWpVXqHbmLeW2Cg/AZ0fX X-Received: by 2002:a17:906:6a0b:b0:b98:14e:32c0 with SMTP id a640c23a62f3a-b9c46e585d0mr167857466b.14.1775154729953; Thu, 02 Apr 2026 11:32:09 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20260324191337.1841376-1-jstultz@google.com> <36e96f87-a682-436e-aefc-13e2e5810019@amd.com> <20260327114844.GQ2872@noisy.programming.kicks-ass.net> <33e60181-1809-44e1-bc4c-8ac7f79d49d6@amd.com> <20260327160017.GK3738010@noisy.programming.kicks-ass.net> <1515d405-62fc-4952-842f-b69e2bf192c0@amd.com> <20260402155055.GV3738010@noisy.programming.kicks-ass.net> In-Reply-To: <20260402155055.GV3738010@noisy.programming.kicks-ass.net> From: John Stultz Date: Thu, 2 Apr 2026 11:31:56 -0700 X-Gm-Features: AQROBzBTUS9PsBmIrr3CbKcPuQnephkQ7UWutyin9Ij0whK2TwBTkOONjfe1Pk4 Message-ID: Subject: Re: [PATCH v26 00/10] Simple Donor Migration for Proxy Execution To: Peter Zijlstra Cc: K Prateek Nayak , LKML , Joel Fernandes , Qais Yousef , Ingo Molnar , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Valentin Schneider , Steven Rostedt , Ben Segall , Zimuzo Ezeozue , Mel Gorman , Will Deacon , Waiman Long , Boqun Feng , "Paul E. McKenney" , Metin Kaya , Xuewen Yan , Thomas Gleixner , Daniel Lezcano , Suleiman Souhlal , kuyo chang , hupu , kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Apr 2, 2026 at 8:51=E2=80=AFAM Peter Zijlstra wrote: > > On Fri, Mar 27, 2026 at 10:27:08PM +0530, K Prateek Nayak wrote: > > > So taking a step back, this is what we have today (at least the > > common scenario): > > > > CPU0 (donor - A) CPU1 (owner - B) > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > mutex_lock() > > __set_current_state(TASK_INTERRUPTIBLE) > > __set_task_blocked_on(M) > > schedule() > > /* Retained for proxy */ > > proxy_migrate_task() > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D> /* Migrates to CPU1 */ > > ... > > send_sig(B) > > signal_wake_up_state() > > wake_up_state() > > try_to_wake_up() > > ttwu_runnable() > > ttwu_do_wakeup() =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D> /= * A->__state =3D TASK_RUNNING */ > > > > /* > > * After this point ttwu_st= ate_match() > > * will fail for A so a mut= ex_unlock() > > * will have to go through = __schedule() > > * for return migration. > > */ > > > > __schedule() > > find_proxy_task() > > > > /* Scenario 1 - B sleep= s */ > > __clear_task_blocked_on= () > > proxy_deactivate(A) > > /* A->__state =3D=3D = TASK_RUNNING */ > > /* fallthrough */ > > > > /* Scenario 2 - return = migration after unlock() */ > > __clear_task_blocked_on= () > > /* > > * At this point proxy = stops. > > * Much later after sig= nal. > > */ > > proxy_force_return() > > schedule() <=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > signal_pending_state() > > > > clear_task_blocked_on() > > __set_current_state(TASK_RUNNING) > > > > ... /* return with -EINR */ > > > > > > Basically, a blocked donor has to wait for a mutex_unlock() before it > > can go process the signal and bail out on the mutex_lock_interruptible(= ) > > which seems counter productive - but it is still okay from correctness > > perspective. > > > > > > > > One thing you *can* do it frob ttwu_runnable() to 'refuse' to wake th= e > > > task, and then it goes into the normal path and will do the migration= . > > > I've done things like that before. > > > > > > Does that fix all the return-migration cases? > > > > Yes it does! If we handle the return via ttwu_runnable(), which is what > > proxy_needs_return() in the next chunk of changes aims to do and we can > > build the invariant that TASK_RUNNING + task_is_blocked() is an illegal > > state outside of __schedule() which works well with ttwu_state_match(). > > > > > > > >> 2. Why does proxy_needs_return() (this comes later in John's tree bu= t I > > >> moved it up ahead) need the proxy_task_runnable_but_waking() ove= rride > > >> of the ttwu_state_mach() machinery? > > >> (https://github.com/johnstultz-work/linux-dev/commit/28ad4d3fa847= b90713ca18a623d1ee7f73b648d9) > > > > > > Since it comes later, I've not seen it and not given it thought ;-) > > > > > > (I mean, I've probably seen it at some point, but being the gold-fish > > > that I am, I have no recollection, so I might as well not have seen i= t). > > > > > > A brief look now makes me confused. The comment fails to describe how > > > that situation could ever come to pass. > > > > That is a signal delivery happening before unlock which will force > > TASK_RUNNING but since we are waiting on an unlock, the wakeup from > > unlock will see TASK_RUNNING + PROXY_WAKING. > > > > We then later force it on the ttwu path to do return via > > ttwu_runnable(). > > So, I've not gone through all the cases yet, and it is *COMPLETELY* > untested, but something like the below perhaps? > So, just to clarify, this suggestion is as an alternative to my return-migration via ttwu logic (not included in the v26 simple-donor-migration chunk you've queued)? https://github.com/johnstultz-work/linux-dev/commit/dfaa472f2a0b2f6a0c73= 083aaba5c55e256fdb56 I'm working on prepping that next chunk of the series (which includes that logic) to send out here shortly (integrating the few bits I from Prateek that I've managerd to get my head around). There's still a bunch of other changes tied into waking a rq->donor outside of __schedule() in that chunk, so I'm not sure if this discussion would be easier to have in context once those are on the list? So some of my (certainly confused) thoughts below... > --- > include/linux/sched.h | 2 > kernel/sched/core.c | 173 ++++++++++++++++---------------------------= ------- > 2 files changed, 58 insertions(+), 117 deletions(-) > > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -161,7 +161,7 @@ struct user_event_mm; > */ > #define is_special_task_state(state) \ > ((state) & (__TASK_STOPPED | __TASK_TRACED | TASK_PARKED | \ > - TASK_DEAD | TASK_FROZEN)) > + TASK_DEAD | TASK_WAKING | TASK_FROZEN)) > > #ifdef CONFIG_DEBUG_ATOMIC_SLEEP > # define debug_normal_state_change(state_value) = \ > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c ... > @@ -3702,28 +3723,39 @@ ttwu_do_activate(struct rq *rq, struct t > */ > static int ttwu_runnable(struct task_struct *p, int wake_flags) > { > - struct rq_flags rf; > - struct rq *rq; > - int ret =3D 0; > + ACQUIRE(__task_rq_lock, guard)(p); > + struct rq *rq =3D guard.rq; > > - rq =3D __task_rq_lock(p, &rf); > - if (task_on_rq_queued(p)) { > - update_rq_clock(rq); > - if (p->se.sched_delayed) > - enqueue_task(rq, p, ENQUEUE_NOCLOCK | ENQUEUE_DEL= AYED); > - if (!task_on_cpu(rq, p)) { > + if (!task_on_rq_queued(p)) > + return 0; > + > + if (sched_proxy_exec() && p->blocked_on) { > + guard(raw_spinlock)(&p->blocked_lock); > + struct mutex *lock =3D p->blocked_on; > + if (lock) { > /* > - * When on_rq && !on_cpu the task is preempted, s= ee if > - * it should preempt the task that is current now= . > + * TASK_WAKING is a special state and results in > + * DEQUEUE_SPECIAL such that the task will actual= ly be > + * forced from the runqueue. > */ > - wakeup_preempt(rq, p, wake_flags); > + block_task(rq, p, TASK_WAKING); > + p->blocked_on =3D NULL; > + return 0; > } > - ttwu_do_wakeup(p); > - ret =3D 1; > } > - __task_rq_unlock(rq, p, &rf); > > - return ret; > + update_rq_clock(rq); > + if (p->se.sched_delayed) > + enqueue_task(rq, p, ENQUEUE_NOCLOCK | ENQUEUE_DELAYED); I can't precisely remember the details now, but I believe we need to handle enqueueing sched_delayed tasks before handling blocked_on tasks. > + if (!task_on_cpu(rq, p)) { > + /* > + * When on_rq && !on_cpu the task is preempted, see if > + * it should preempt the task that is current now. > + */ > + wakeup_preempt(rq, p, wake_flags); > + } > + ttwu_do_wakeup(p); > + return 1; > } > > void sched_ttwu_pending(void *arg) ... > @@ -6586,13 +6598,12 @@ static inline struct task_struct *proxy_ > return rq->idle; > } > > -static bool proxy_deactivate(struct rq *rq, struct task_struct *donor) > +static void proxy_deactivate(struct rq *rq, struct task_struct *donor) > { > unsigned long state =3D READ_ONCE(donor->__state); > > - /* Don't deactivate if the state has been changed to TASK_RUNNING= */ > - if (state =3D=3D TASK_RUNNING) > - return false; > + WARN_ON_ONCE(state =3D=3D TASK_RUNNING); > + > /* > * Because we got donor from pick_next_task(), it is *crucial* > * that we call proxy_resched_idle() before we deactivate it. > @@ -6603,7 +6614,7 @@ static bool proxy_deactivate(struct rq * > * need to be changed from next *before* we deactivate. > */ > proxy_resched_idle(rq); > - return try_to_block_task(rq, donor, &state, true); > + block_task(rq, donor, state); > } > > static inline void proxy_release_rq_lock(struct rq *rq, struct rq_flags = *rf) > @@ -6677,71 +6688,6 @@ static void proxy_migrate_task(struct rq > proxy_reacquire_rq_lock(rq, rf); > } > > -static void proxy_force_return(struct rq *rq, struct rq_flags *rf, > - struct task_struct *p) > - __must_hold(__rq_lockp(rq)) > -{ > - struct rq *task_rq, *target_rq =3D NULL; > - int cpu, wake_flag =3D WF_TTWU; > - > - lockdep_assert_rq_held(rq); > - WARN_ON(p =3D=3D rq->curr); > - > - if (p =3D=3D rq->donor) > - proxy_resched_idle(rq); > - > - proxy_release_rq_lock(rq, rf); > - /* > - * We drop the rq lock, and re-grab task_rq_lock to get > - * the pi_lock (needed for select_task_rq) as well. > - */ > - scoped_guard (task_rq_lock, p) { > - task_rq =3D scope.rq; > - > - /* > - * Since we let go of the rq lock, the task may have been > - * woken or migrated to another rq before we got the > - * task_rq_lock. So re-check we're on the same RQ. If > - * not, the task has already been migrated and that CPU > - * will handle any futher migrations. > - */ > - if (task_rq !=3D rq) > - break; > - > - /* > - * Similarly, if we've been dequeued, someone else will > - * wake us > - */ > - if (!task_on_rq_queued(p)) > - break; > - > - /* > - * Since we should only be calling here from __schedule() > - * -> find_proxy_task(), no one else should have > - * assigned current out from under us. But check and warn > - * if we see this, then bail. > - */ > - if (task_current(task_rq, p) || task_on_cpu(task_rq, p)) = { > - WARN_ONCE(1, "%s rq: %i current/on_cpu task %s %d= on_cpu: %i\n", > - __func__, cpu_of(task_rq), > - p->comm, p->pid, p->on_cpu); > - break; > - } > - > - update_rq_clock(task_rq); > - deactivate_task(task_rq, p, DEQUEUE_NOCLOCK); > - cpu =3D select_task_rq(p, p->wake_cpu, &wake_flag); > - set_task_cpu(p, cpu); > - target_rq =3D cpu_rq(cpu); > - clear_task_blocked_on(p, NULL); > - } > - > - if (target_rq) > - attach_one_task(target_rq, p); > - > - proxy_reacquire_rq_lock(rq, rf); > -} > - > /* > * Find runnable lock owner to proxy for mutex blocked donor > * > @@ -6777,7 +6723,7 @@ find_proxy_task(struct rq *rq, struct ta > clear_task_blocked_on(p, PROXY_WAKING); > return p; > } > - goto force_return; > + goto deactivate; > } > > /* > @@ -6812,7 +6758,7 @@ find_proxy_task(struct rq *rq, struct ta > __clear_task_blocked_on(p, NULL); > return p; > } > - goto force_return; > + goto deactivate; > } > > if (!READ_ONCE(owner->on_rq) || owner->se.sched_delayed) = { > @@ -6891,12 +6837,7 @@ find_proxy_task(struct rq *rq, struct ta > return owner; > > deactivate: > - if (proxy_deactivate(rq, donor)) > - return NULL; > - /* If deactivate fails, force return */ > - p =3D donor; > -force_return: > - proxy_force_return(rq, rf, p); > + proxy_deactivate(rq, donor); > return NULL; > migrate_task: > proxy_migrate_task(rq, rf, p, owner_cpu); So I like getting rid of proxy_force_return(), but its not clear to me that proxy_deactivate() is what we want to do in these find_proxy_task() edge cases. It feels like if we are already racing with ttwu, deactivating the task seems like it might open more windows where we might lose the wakeup. In fact, the whole reason we have proxy_force_return() is that earlier in the proxy-exec development, when we hit those edge cases we usually would return proxy_reschedule_idle() just to drop the rq lock and let ttwu do its thing, but there kept on being cases where we would end up with lost wakeups. But I'll give this a shot (and will integrate your ttwu_runnable cleanups regardless) and see how it does. thanks -john