Messages in this thread | | | From | Vincent Guittot <> | Date | Fri, 6 Dec 2019 08:57:11 +0100 | Subject | Re: [PATCH v2] sched/fair: Do not set skip buddy up the sched hierarchy |
| |
Hi Josh,
On Wed, 4 Dec 2019 at 21:06, Josh Don <joshdon@google.com> wrote: > > From: Venkatesh Pallipadi <venki@google.com> > > Setting skip buddy all the way up the hierarchy does not play well > with intra-cgroup yield. One typical usecase of yield is when a > thread in a cgroup wants to yield CPU to another thread within the > same cgroup. For such a case, setting the skip buddy all the way up > the hierarchy is counter-productive, as that results in CPU being > yielded to a task in some other cgroup. > > So, limit the skip effect only to the task requesting it. > > Signed-off-by: Josh Don <joshdon@google.com>
There is a mismatch between the author Venkatesh Pallipadi and the signoff Josh Don If Venkatesh is the original author and you have then done some modifications, your both signed-off should be there
Apart from that, the change makes sense to me
> --- > v2: Only clear skip buddy on the current cfs_rq > > kernel/sched/fair.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 08a233e97a01..0b7a1958ad52 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -4051,13 +4051,10 @@ static void __clear_buddies_next(struct sched_entity *se) > > static void __clear_buddies_skip(struct sched_entity *se) > { > - for_each_sched_entity(se) { > - struct cfs_rq *cfs_rq = cfs_rq_of(se); > - if (cfs_rq->skip != se) > - break; > + struct cfs_rq *cfs_rq = cfs_rq_of(se); > > + if (cfs_rq->skip == se) > cfs_rq->skip = NULL; > - } > } > > static void clear_buddies(struct cfs_rq *cfs_rq, struct sched_entity *se) > @@ -6552,8 +6549,15 @@ static void set_next_buddy(struct sched_entity *se) > > static void set_skip_buddy(struct sched_entity *se) > { > - for_each_sched_entity(se) > - cfs_rq_of(se)->skip = se; > + /* > + * One typical usecase of yield is when a thread in a cgroup > + * wants to yield CPU to another thread within the same cgroup. > + * For such a case, setting the skip buddy all the way up the > + * hierarchy is counter-productive, as that results in CPU being > + * yielded to a task in some other cgroup. So, only set skip > + * for the task requesting it. > + */ > + cfs_rq_of(se)->skip = se; > } > > /* > -- > 2.24.0.393.g34dc348eaf-goog >
| |