Received-SPF: pass (google.com: domain of linux-kernel+bounces-153502-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 22 Apr 2024 21:52:19 +0800
From: Chen Yu <yu.c.chen@intel.com>
To: Xuewen Yan <xuewen.yan94@gmail.com>
CC: Peter Zijlstra <peterz@infradead.org>, Xuewen Yan <xuewen.yan@unisoc.com>,
	<mingo@redhat.com>, <juri.lelli@redhat.com>, <vincent.guittot@linaro.org>,
	<dietmar.eggemann@arm.com>, <rostedt@goodmis.org>, <bsegall@google.com>,
	<mgorman@suse.de>, <bristot@redhat.com>, <vschneid@redhat.com>,
	<ke.wang@unisoc.com>, <di.shen@unisoc.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] sched/eevdf: Prevent vlag from going out of bounds
 when reweight_eevdf
Message-ID: <ZiZrkyzQQQny945u@chenyu5-mobl2>
References: <20240422082238.5784-1-xuewen.yan@unisoc.com>
 <CAB8ipk-LhSuMsp0DdzjEJN-4XEBT1_ri6BPH_nvxSgFZONMT2Q@mail.gmail.com>
 <20240422094157.GA34453@noisy.programming.kicks-ass.net>
 <CAB8ipk9M1wLXV7MWMBYGMhKbY71QqwkvchiENeWTg7JERe6kZw@mail.gmail.com>
 <20240422111744.GO30852@noisy.programming.kicks-ass.net>
 <CAB8ipk8ZBSnJfw9Ow9My-VXG1TU6DVY7mOL_i0Ejdd7GUZCLWA@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAB8ipk8ZBSnJfw9Ow9My-VXG1TU6DVY7mOL_i0Ejdd7GUZCLWA@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bklxSm8vdWhZUGpRcGR3V0pvK2MrbVdla0Zsb3NoelZleFRCUm1zWkxvQlpW?=
 =?utf-8?B?STlxU0pkY0ZTZ0k5cUhlTWRJNzJJeUFqd2piUmJFVTVQcFpGdkhKUGUvRU4w?=
 =?utf-8?B?Y0pZWWJSR1lZd2dXR2FDUzJ4a1E4aXR3aEVvS0tpem82SDFJUFJ1cTdhQ00x?=
 =?utf-8?B?eVRrRnM0ZmhIbW1uWmthQ2t6Y0dURWwrWWRUSjFRMTRLaUlqQkIrSWdKcitT?=
 =?utf-8?B?SXpIcUovdm5UWVprY21SWkJrQU04dmVJQmt0Ry8xSXBSQTA5UzhKdjB3UURl?=
 =?utf-8?B?UHVQSFlOSW5iN0xtWkViMW1NS1ZTQnp2eE9kZlN6bWpPVENFVWtPbWQ2bHpU?=
 =?utf-8?B?WlhlaEhYRjliNTdnL010R3NFOSt4V010dWdPSEx0VXNWRmVzSi90Nk9DZXZ5?=
 =?utf-8?B?Q2ZRamRiNkpGTTJhN1FYUm9qYXFtdEUxTXNlTThWNDZ2eTRJbEtubFVLd0hx?=
 =?utf-8?B?Y3hiY2hBS05ZQTFnUmhVaDZITk5KZWlwSXpGUFNQVDFxT2xIeHhibWI3T0Vn?=
 =?utf-8?B?c2V4RHpFMWlZTitHUjJOUG1NSm45aWNCQThSL1pRVW9JdTVKOXhja3NQTWxm?=
 =?utf-8?B?WHNhaEgvV2xHT2hlQnNybGJTWUxlSFBGb0x0OFVtb2FBOXQydXUwTytKTTZo?=
 =?utf-8?B?OWdUa2Z1OWNNYU5UWWZoNHNsTUpNb2hYRW5PdEVuZXRCNDh5NmVaUHQ5eDB6?=
 =?utf-8?B?eSt5U012UXQ3L2pIb0tBU0hnM3FGM3hkTVF5NVBZMWtSYksyS2pMaE9iQ1BE?=
 =?utf-8?B?cUQ1Sks3MEVxS3ZDUU1KWmo0QXNuUU9EK2lxSE15TlpkQWMwK2lKWm5iQWFq?=
 =?utf-8?B?RWpGTmFISnR4d3lnRm4wa3JOcC9qQk0zdGhhRDhXVG9GVmpNdnhVbnIvdFp3?=
 =?utf-8?B?WUl3dWlEZUtadzkvQi90anhlZ1BtTEhwTWZvZFF0d1Nxbk84d1BBRkNKbkJj?=
 =?utf-8?B?K0NETWRMcTQ1bXpPaHNCMElNaEpiSnUyekNSVmJ3VmJRcC9IWjk5WWQ3T3cw?=
 =?utf-8?B?dzM3cUhTSXBnN3d0bzhzRHgvTmJTQVNGZnRPUks4cVZxa3Y5eTFqc3Q1OWpR?=
 =?utf-8?B?TmVub2dWQjVDdytmOTcvRm85OFl6SnZISDRabW1PdHZ2UlRSS0U5K1NPYzhD?=
 =?utf-8?B?U1NzdHZwN3RNWU82M0RDSThHd1RoNXNNVnJLcGV6R2dmaGNyYzdmYmxIOHM4?=
 =?utf-8?B?ZUtJNGFwQmVtdTJuNE54V3pRbHRkWnlDUGhkTlkydHZnRnhlbUtLRmQ3R1hl?=
 =?utf-8?B?YXg4andqc2kraWRsQldPK1FTY3JtV2xwOW5ONW1NNDdQMDBhd2JYaUpUaS9z?=
 =?utf-8?B?NXdPYkJ0VGM3QWF0QzQ1RUJBcXhVSS93bVQrbGtDVFN6alV2QXQxbXhpc0tT?=
 =?utf-8?B?ZHEzZkdMaHF6d1BiK1EvZHp1S2FIR2t6dkFxa3NNSHFUY1BKLzFCQWgwZHV3?=
 =?utf-8?B?U3ZseWJXOXAvNWl4VHdqTUcrT2hqZ0JweVZuSWhGb3ZiZ2VTL1F2clJ6WVhu?=
 =?utf-8?B?WkZTdmlvZjB6alFFT1JNYllWa0tVMEc0dnpWeW9nTlcyNmVXL1JWVDRyTGwy?=
 =?utf-8?B?d3Q2QnFvVS9STktOS3ByRVB4cGJ4N3MxMzVHWTJ6dUdseUlJckNTZnBubS9Z?=
 =?utf-8?B?MlpPZXdabjZJbmlHOFQrNGRDYXVkUWo0U1FoaC9PMU5ubjhFMVMvNUErUFJ5?=
 =?utf-8?B?NGllSURzdkRiY1NDRzg3R0tuN2FFcFA4OFRFSGxsK1ZWNWR3UHNNNy84clZ1?=
 =?utf-8?B?cjBKRStuT0VkN1JnWFhLYkxWVmJGUXlPRHo2V1BBSmtQTzM1T1I1Y3pDNzQz?=
 =?utf-8?B?ODE1Qi9VdlNBdnlySjZMWjZObENWcG4reGRFNTROODgyb0xzenpQN0p0T2hX?=
 =?utf-8?B?eTRTb3dxMVB2Yk16Z21vRk1OUzUwMk9Kc1F5a0JLZVhqOWVGT2lEVVdlazQw?=
 =?utf-8?B?aHZ0bTJXTE5mMllLRmNWTTdWbElhM3IzVmRKYnlRVTFSM2NQV1h5OHBBUGp6?=
 =?utf-8?B?OFU3ci9IbVlSQlZIc1JGTlpKS0ZXOFZwRmIvTS9Bc0EwTXBIZml3ZkdUbEl1?=
 =?utf-8?B?NnZUd2NZWnlRWEdSaFhPdDFHSkRBcWU4VmRQbUFnMkhYM0VwemRlVDVRblNk?=
 =?utf-8?Q?vI8tiKzV6JJ/ZK/Y87Ipo09eL?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8bd6a082-fc48-44e0-9a8c-08dc62d372ec
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6020.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2024 13:52:28.7287
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +t1lVw6pi7qSOn9AScHiFYEgBR8Sm0d0JkPoXuONIuCs95bUqp1oACX+cKqi1fQ5h67019VUfwZD/GAQptFV3Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB8002
X-OriginatorOrg: intel.com

On 2024-04-22 at 21:12:12 +0800, Xuewen Yan wrote:
> Hi peter,
> 
> On Mon, Apr 22, 2024 at 7:17 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Mon, Apr 22, 2024 at 07:07:25PM +0800, Xuewen Yan wrote:
> > > On Mon, Apr 22, 2024 at 5:42 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > On Mon, Apr 22, 2024 at 04:33:37PM +0800, Xuewen Yan wrote:
> > > >
> > > > > On the Android system, the nice value of a task will change very
> > > > > frequently. The limit can also be exceeded.
> > > > > Maybe the !on_rq case is still necessary.
> > > > > So I'm planning to propose another patch for !on_rq case later after
> > > > > careful testing locally.
> > > >
> > > > So the scaling is: vlag = vlag * old_Weight / weight
> > > >
> > > > But given that integer devision is truncating, you could expect repeated
> > > > application of such scaling would eventually decrease the vlag instead
> > > > of grow it.
> > > >
> > > > Is there perhaps an invocation of reweight_task() missing? Looking at
> > >
> > > Is it necessary to add reweight_task in the prio_changed_fair()?
> >
> > I think that's the wrong place. Note how __setscheduler_params() already
> > has set_load_weight(). And all other callers of ->prio_changed() already
> > seem to do set_load_weight() as well.
> >
> > But that idle policy thing there still looks wrong, that sets the weight
> > very low but doesn't re-adjust anything.
> 
> By adding a log to observe weight changes in reweight_entity, I found
> that calc_group_shares() often causes new_weight to become very small:
>

If I understand correctly, the on_rq matters when doing reweight.
In the following calltrace, after the entity(task group) is dequeued from
the tree, on_rq is 0, then subsequent update_cfs_group()->reweight_entity()
does not clamp the vlag because reweight_eevdf() can not be invoked, which could result in
the scaling(237238/2) of se->vlag quite large.

thanks,
Chenyu
 
> Hardware name: Unisoc UMS-base Board (DT)
> Call trace:
> dump_backtrace+0xec/0x138
> show_stack+0x18/0x24
> dump_stack_lvl+0x60/0x84
> dump_stack+0x18/0x24
> reweight_entity+0x3e8/0x5f4
> dequeue_task_fair+0x448/0x948
> dequeue_task+0xc4/0x398
> deactivate_task+0x1c/0x28
> pull_tasks+0x200/0x334
> newidle_balance+0x3cc/0x438
> pick_next_task_fair+0x58/0x670
> __schedule+0x204/0x9a0
> schedule+0x128/0x1a8
> schedule_timeout+0x44/0x1c8
> __skb_wait_for_more_packets+0xd0/0x17c
> __unix_dgram_recvmsg+0xdc/0x3a8
> unix_seqpacket_recvmsg+0x64/0x74
> __sys_recvfrom+0x14c/0x1e4
> __arm64_sys_recvfrom+0x24/0x38
> invoke_syscall+0x58/0x114
> el0_svc_common+0xac/0xe0
> do_el0_svc+0x1c/0x28
> el0_svc+0x3c/0x70
> el0t_64_sync_handler+0x68/0xbc
> el0t_64_sync+0x1a8/0x1ac
> reweight_entity: the lag=-831088603030 vruntime=2086205903
> limit=3071999998 old_weight=237238 new_weight=2