Messages in this thread | | | From | Josh Don <> | Date | Fri, 6 Dec 2019 14:13:15 -0800 | Subject | Re: [PATCH v2] sched/fair: Do not set skip buddy up the sched hierarchy |
| |
Hi Vincent,
Thanks for taking a look.
> 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
Venkatesh no longer works at Google, so I don't have a way to get in touch with him. Is my signed-off insufficient for this case?
On Thu, Dec 5, 2019 at 11:57 PM Vincent Guittot <vincent.guittot@linaro.org> wrote: > > 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 > >
| |