Messages in this thread Patch in this message | | | Date | Tue, 23 Apr 2024 09:26:34 +0800 | From | Yujie Liu <> | Subject | Re: [PATCH v2] sched/eevdf: Prevent vlag from going out of bounds when reweight_eevdf |
| |
On Mon, Apr 22, 2024 at 04:47:31PM +0800, Chen Yu wrote: > On 2024-04-22 at 16:22:38 +0800, Xuewen Yan wrote: > > kernel encounters the following error when running workload: > > > > BUG: kernel NULL pointer dereference, address: 0000002c > > EIP: set_next_entity (fair.c:?) > > > > which was caused by NULL pointer returned by pick_eevdf(). > > > > Further investigation has shown that, the entity_eligible() has an > > false-negative issue when the entity's vruntime is far behind the > > cfs_rq.min_vruntime that, the (vruntime - cfs_rq->min_vruntime) * load > > caused a s64 overflow, thus every entity on the rb-tree is not > > eligible, which results in a NULL candidate. > > > > The reason why entity's vruntime is far behind the cfs_rq.min_vruntime > > is because during a on_rq task group's update_cfs_group()->reweight_eevdf(), > > there is no limit on the new entity's vlag. If the new weight is much > > smaller than the old one, > > > > vlag = div_s64(vlag * old_weight, weight) > > > > generates a huge vlag, and results in very small(negative) vruntime. > > > > Thus limit the range of vlag accordingly. > > > > Thanks for the fix. > > Might also add comments from Tim suggested here: > https://lore.kernel.org/lkml/ec479251e6245148b89b226f734192f6d5343bbc.camel@linux.intel.com/ > > A fix tag might be needed. > Fixes: eab03c23c2a1 ("sched/eevdf: Fix vruntime adjustment on reweight") > > > Reported-by: Sergei Trofimovich <slyich@gmail.com> > > Closes: https://lore.kernel.org/all/ZhuYyrh3mweP_Kd8@nz.home/ > > Reported-by: Igor Raits <igor@gooddata.com> > > Closes: https://lore.kernel.org/all/CA+9S74ih+45M_2TPUY_mPPVDhNvyYfy1J1ftSix+KjiTVxg8nw@mail.gmail.com/ > > Reported-by: Breno Leitao <leitao@debian.org> > > Reported-by: kernel test robot <oliver.sang@intel.com> > > Closes: https://lore.kernel.org/lkml/202401301012.2ed95df0-oliver.sang@intel.com/ > > Reported-by: Yujie Liu <yujie.liu@intel.com> > > Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com> > > --- > > Cced Sergei, Igor, Breno who have encountered this NULL pointer issue before. > > From my testing, with this applied I did not see the NULL pointer exception > after running trinity for 100 cycles, so > > Reviewed-and-tested-by: Chen Yu <yu.c.chen@intel.com> > > thanks, > Chenyu >
From 0-Day testing, with this patch applied on v6.9-rc5, we no longer see the NULL pointer issue in 999 cycles of trinity test.
Tested-by: Yujie Liu <yujie.liu@intel.com>
========================================================================================= tbox_group/testcase/rootfs/kconfig/compiler/runtime/group/nr_groups: vm-snb/trinity/debian-11.1-i386-20220923.cgz/i386-randconfig-004-20240122/clang-17/300s/group-03/5
commit: v6.9-rc5 v6.9-rc5+patch ("sched/eevdf: Prevent vlag from going out of bounds when reweight_eevdf")
v6.9-rc5 v6.9-rc5+patch ---------------- --------------------------- fail:runs %reproduction fail:runs | | | 41:999 -4% :999 dmesg.BUG:kernel_NULL_pointer_dereference,address 24:999 -2% :999 dmesg.EIP:pick_next_task_fair 17:999 -2% :999 dmesg.EIP:set_next_entity 41:999 -4% :999 dmesg.Kernel_panic-not_syncing:Fatal_exception 41:999 -4% :999 dmesg.Oops:#[##]
> > changes of v2: > > -add reported-by (suggested by <yu.c.chen@intel.com>) > > -remork the changelog (<yu.c.chen@intel.com>) > > -remove the judgement of fork (Peter) > > -remove the !on_rq case. (Peter) > > --- > > Previous discussion link: > > https://lore.kernel.org/lkml/20240226082349.302363-1-yu.c.chen@intel.com/ > > https://lore.kernel.org/all/20240130080643.1828-1-xuewen.yan@unisoc.com/ > > --- > > --- > > kernel/sched/fair.c | 18 ++++++++++++------ > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index 03be0d1330a6..64826f406d6d 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -696,15 +696,21 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq) > > * > > * XXX could add max_slice to the augmented data to track this. > > */ > > -static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se) > > +static s64 entity_lag(u64 avruntime, struct sched_entity *se) > > { > > - s64 lag, limit; > > + s64 vlag, limit; > > + > > + vlag = avruntime - se->vruntime; > > + limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se); > > + > > + return clamp(vlag, -limit, limit); > > +} > > > > +static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se) > > +{ > > SCHED_WARN_ON(!se->on_rq); > > - lag = avg_vruntime(cfs_rq) - se->vruntime; > > > > - limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se); > > - se->vlag = clamp(lag, -limit, limit); > > + se->vlag = entity_lag(avg_vruntime(cfs_rq), se); > > } > > > > /* > > @@ -3761,7 +3767,7 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se, > > * = V - vl' > > */ > > if (avruntime != se->vruntime) { > > - vlag = (s64)(avruntime - se->vruntime); > > + vlag = entity_lag(avruntime, se); > > vlag = div_s64(vlag * old_weight, weight); > > se->vruntime = avruntime - vlag; > > } > > -- > > 2.25.1 > >
| |