All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Vernet <void@manifault.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, mingo@redhat.com,
	juri.lelli@redhat.com, vincent.guittot@linaro.org,
	rostedt@goodmis.org, dietmar.eggemann@arm.com,
	bsegall@google.com, mgorman@suse.de, bristot@redhat.com,
	vschneid@redhat.com, joshdon@google.com,
	roman.gushchin@linux.dev, tj@kernel.org, kernel-team@meta.com
Subject: Re: [RFC PATCH 2/3] sched/fair: Add SWQUEUE sched feature and skeleton calls
Date: Thu, 22 Jun 2023 09:53:52 -0500	[thread overview]
Message-ID: <20230622145352.GB113759@maniforge> (raw)
In-Reply-To: <20230621124933.GE2053369@hirez.programming.kicks-ass.net>

On Wed, Jun 21, 2023 at 02:49:33PM +0200, Peter Zijlstra wrote:
> On Tue, Jun 13, 2023 at 12:20:03AM -0500, David Vernet wrote:
> 
> I can't help but read this thing as software-queue :/ Can we please pick
> a better name?

Yes, I'll think of a better one. Suggestions welcome if you have a
preference.

> > @@ -6368,6 +6390,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> >  	if (!task_new)
> >  		update_overutilized_status(rq);
> >  
> > +	if (sched_feat(SWQUEUE))
> > +		swqueue_enqueue(rq, p, flags);
> > +
> >  enqueue_throttle:
> >  	assert_list_leaf_cfs_rq(rq);
> >  
> > @@ -6449,6 +6474,9 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
> >  dequeue_throttle:
> >  	util_est_update(&rq->cfs, p, task_sleep);
> >  	hrtick_update(rq);
> > +
> > +	if (sched_feat(SWQUEUE))
> > +		swqueue_remove_task(p);
> >  }
> >  
> >  #ifdef CONFIG_SMP
> 
> _enqueue() should obviously be complemented by _dequeue(). This naming
> is offensive :-)

Ack

> > @@ -8155,12 +8183,18 @@ done: __maybe_unused;
> >  
> >  	update_misfit_status(p, rq);
> >  
> > +	if (sched_feat(SWQUEUE))
> > +		swqueue_remove_task(p);
> > +
> >  	return p;
> >  
> >  idle:
> >  	if (!rf)
> >  		return NULL;
> >  
> > +	if (sched_feat(SWQUEUE) && swqueue_pick_next_task(rq, rf))
> > +		return RETRY_TASK;
> > +
> >  	new_tasks = newidle_balance(rq, rf);
> >  
> >  	/*
> 
> That's either not correct or insufficient or both.
> 
> It fails to consider the whole core-scheduling mess. But it also fails
> to consider the regular (non optimized) pick case that should do newidle
> through put_prev_task_balance() -> balance_fair().
> 
> I think placing the pick call in newidle_balance() itself is the
> simplest solution.

Yep, not sure where I went off the rails here -- the pick call clearly
belongs in newidle_balance(). I'll also make sure we handle core sched
correctly as well. Thanks for pointing those out.

  reply	other threads:[~2023-06-22 14:54 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13  5:20 [RFC PATCH 0/3] sched: Implement shared wakequeue in CFS David Vernet
2023-06-13  5:20 ` [RFC PATCH 1/3] sched: Make migrate_task_to() take any task David Vernet
2023-06-21 13:04   ` Peter Zijlstra
2023-06-22  2:07     ` David Vernet
2023-06-13  5:20 ` [RFC PATCH 2/3] sched/fair: Add SWQUEUE sched feature and skeleton calls David Vernet
2023-06-21 12:49   ` Peter Zijlstra
2023-06-22 14:53     ` David Vernet [this message]
2023-06-13  5:20 ` [RFC PATCH 3/3] sched: Implement shared wakequeue in CFS David Vernet
2023-06-13  8:32   ` Peter Zijlstra
2023-06-14  4:35     ` Aaron Lu
2023-06-14  9:27       ` Peter Zijlstra
2023-06-15  0:01       ` David Vernet
2023-06-15  4:49         ` Aaron Lu
2023-06-15  7:31           ` Aaron Lu
2023-06-15 23:26             ` David Vernet
2023-06-16  0:53               ` Aaron Lu
2023-06-20 17:36                 ` David Vernet
2023-06-21  2:35                   ` Aaron Lu
2023-06-21  2:43                     ` David Vernet
2023-06-21  4:54                       ` Aaron Lu
2023-06-21  5:43                         ` David Vernet
2023-06-21  6:03                           ` Aaron Lu
2023-06-22 15:57                             ` Chris Mason
2023-06-13  8:41   ` Peter Zijlstra
2023-06-14 20:26     ` David Vernet
2023-06-16  8:08   ` Vincent Guittot
2023-06-20 19:54     ` David Vernet
2023-06-20 21:37       ` Roman Gushchin
2023-06-21 14:22       ` Peter Zijlstra
2023-06-19  6:13   ` Gautham R. Shenoy
2023-06-20 20:08     ` David Vernet
2023-06-21  8:17       ` Gautham R. Shenoy
2023-06-22  1:43         ` David Vernet
2023-06-22  9:11           ` Gautham R. Shenoy
2023-06-22 10:29             ` Peter Zijlstra
2023-06-23  9:50               ` Gautham R. Shenoy
2023-06-26  6:04                 ` Gautham R. Shenoy
2023-06-27  3:17                   ` David Vernet
2023-06-27 16:31                     ` Chris Mason
2023-06-21 14:20   ` Peter Zijlstra
2023-06-21 20:34     ` David Vernet
2023-06-22 10:58       ` Peter Zijlstra
2023-06-22 14:43         ` David Vernet
2023-07-10 11:57 ` [RFC PATCH 0/3] " K Prateek Nayak
2023-07-11  4:43   ` David Vernet
2023-07-11  5:06     ` K Prateek Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230622145352.GB113759@maniforge \
    --to=void@manifault.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joshdon@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=roman.gushchin@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.