Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <3f7d41bd-ca12-4458-89ba-474d7a4b5bb2@vivo.com>
Date:   Thu, 14 Sep 2023 20:04:27 +0800
User-Agent: Mozilla Thunderbird
From:   Bixuan Cui <cuibixuan@vivo.com>
Subject: Re: [RFC PATCH v2 2/5] mm: Add policy_name to identify OOM policies
To:     Jonathan Corbet <corbet@lwn.net>,
        Chuyi Zhou <zhouchuyi@bytedance.com>, hannes@cmpxchg.org,
        mhocko@kernel.org, roman.gushchin@linux.dev, ast@kernel.org,
        daniel@iogearbox.net, andrii@kernel.org, muchun.song@linux.dev
Cc:     bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
        wuyun.abel@bytedance.com, robin.lu@bytedance.com
References: <20230810081319.65668-1-zhouchuyi@bytedance.com>
 <20230810081319.65668-3-zhouchuyi@bytedance.com>
 <87h6p1uz3w.fsf@meer.lwn.net>
In-Reply-To: <87h6p1uz3w.fsf@meer.lwn.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bWlRdUJvaGNjNG1GMEIyd1pGdSsrK3VRZWRqVWhoQ3VmK3RkUHUwWDBDRVpi?=
 =?utf-8?B?cmowbEkrNjk0NDVpOVNBK2NwcFJZN2VYM1hzT0R5eHQ3NVc3VnlsL0xMVzVz?=
 =?utf-8?B?Y0VpMjJRaEw1aS9Wc1BKeC9yNCsxY1pGdHZmak1SR2ljQ0lJRURkWHNiNDkv?=
 =?utf-8?B?SUptYjljdGRJVG83bVRvMndkN2JCMTNISzk0UFUzL3JBNnBNMWZxQy9odEZ5?=
 =?utf-8?B?cmpveXhUVVpkUjRyaWtIcXZPMGRRaDdweWc3MDB1U0NXZU9kMjdPc1B3enBl?=
 =?utf-8?B?dXJCZG1PT0N2Tk04RGk1M2FFMXVEM1dDaEVjRUNXT0tHY2lOdk5selhGL2Zl?=
 =?utf-8?B?TFViUXJPR2E4VzNnNmFYTkNMUHA1aXl6NFhaWTJHTTJpMEhXZEZ6emxpeklB?=
 =?utf-8?B?L2lsbzdYb0FaRlFvVVRSaHhUaEdHMjN5WDdqRHd4RVEvcCtUSXFEOTV0NWRN?=
 =?utf-8?B?V3NPR2g2R0xkd1JZVjNwa0w2UXNKK0xIS0VZbzUrQ0lkZ3pqczV4TDNuKzVi?=
 =?utf-8?B?dWxZTVZZZ1RwenJtQ3JySUgyeXNVVEErYTkxTlEvTkNvSm5IMzM1UlNwaGFP?=
 =?utf-8?B?UmRTQU1DejlkNitZVmlMQ0ZsdklQRWNZVHhIbyt1Y281c2oxQVk4M2RFUGRC?=
 =?utf-8?B?N04vMkNMM1NtbU0vSkNMQVBxdXFiSUVCT1o2eTM4dFFLNmdWN2U0SDhwanAy?=
 =?utf-8?B?dXJ2S2dpQ2VtQ2VCemtPdU5zODBFMktWM2dYcGZSeFozWDI4MC9xUWVjZDEv?=
 =?utf-8?B?UXp1RTEra0t0OWhJUkVKcUFKNjRUSFViaWpueTNEb2tEYnRXc0dwQmt3cmMw?=
 =?utf-8?B?cWg5R3VWY09Fd2hSRnkxcGxTOXZmbElYSG1ucS90MXRKVk16bTJseElpNTJ4?=
 =?utf-8?B?ZXNMeHhpNGdqZTVoZ1l3ZlhpQmZmaDljbUFHTFV3dmtxMlRIeWl0R0FSOCs1?=
 =?utf-8?B?V3VBTFp3ZndvdGo1UjFQUFI1c2pFb3pCalZKbXNBSkpJMUg4bUdvM0hIai9v?=
 =?utf-8?B?L0M2cHp1Rm1vV3BoOUczUmxINTZjMXFYK0VieGo4ZllqN0JGeHVVM0ZVMndl?=
 =?utf-8?B?ajgwRWRSWE4zeXY5WGh5eXNNNVlIaVpISlBOc3pvZExzRldKbHp4NnVwSHpi?=
 =?utf-8?B?TFpSeEw5YUhNN01SZXVYQmtIelIyaFA2czZTbXdXZGUrcThTWUx3WDV6alVN?=
 =?utf-8?B?L0ZQOEg1S2pEUFk3bnZ3SFZmWk1DNyt3bG1Ic243SHc5YmhTcGlKZ0JIdUJi?=
 =?utf-8?B?aGNJQ283ajE1RzlkU0t6UG5teFVyQ3QvcHVvdktYck9Uc21nZktMemdsSHhF?=
 =?utf-8?B?R3YvaGxIME0rRUtuUUg5ajVXbWZIUzVhRHhNMnFBWVppWFBwRVFWbzgzTldQ?=
 =?utf-8?B?dmxpWWY1SkdINm1vWk9mS3EvdGZtdzAzZ2hFMXZkWUlra3N0NSt0LzdFWThv?=
 =?utf-8?B?V2xYT2d2SnoyV1FZU3EyTjMrNDRyc2FqTVowL3o0N09qSGpmSk83Q01Tbm5s?=
 =?utf-8?B?ZkthSVdSeWhmNGd1eXE0L0Iyblp5L1laa2M3MUpkWTVEb2wweUxJSGhralh5?=
 =?utf-8?B?aEppZkMwUXN2bHV6UHlueEhDSzFXa1ZaTkF6bG1sS0lLVGRvVC9GQ1d2dnVo?=
 =?utf-8?B?cGlBRlFhVm9BYTlka20wcXBWaytjVE9WRjJtbkpMS0VubE5XRWNxanRJR090?=
 =?utf-8?B?RVF0WDVLUVRWSjJhdWhCT09IbWpvUjZOckxPTGtJZ3g5UEovVHpnNExXa2JS?=
 =?utf-8?B?eGtKakZOcU1KTHZKU0lDamdoa1EyeHJpYVdvWjVVU0JFaFpwZTFCVU9maWUx?=
 =?utf-8?B?WUIxNU5UYXU1VHlxYVNBUU9BMGVDc3VIQjJHcHNzdlN3UGhkb3d2K3hPQ3FG?=
 =?utf-8?B?RUhHNVVaK0pqaEJKWDJNV29tNmVPenJkMGFkZzk0ZFdDL2tKdWpGbElrbjh0?=
 =?utf-8?B?MTROV0d1aWFKWWV1cjJ4RjdZVXl5ZE5oaUhTTEc5V2JNb3dIOGdFUkd6TmhN?=
 =?utf-8?B?NGliWTJTU1k3bEtsclBpTGNsMDkxSjBua0RRZVV1ZW9STUVzVzVVNDJicmp0?=
 =?utf-8?B?THFEaEtnTGRzZ1ptQk1Gb1BDaFl6V3JvcHN4ZVA0dC92cjQ1ZlJmQk5aSVFE?=
 =?utf-8?Q?pWxzDkGbMrM9YunZKE2vXwJao?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 76a5391b-72e2-4dcd-ba49-08dbb51abff0
X-MS-Exchange-CrossTenant-AuthSource: TYZPR06MB4045.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Sep 2023 12:04:29.7819
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8bmOGvsGzoLgr3hq8ZCg5Yw6iQ59wrpomwwUEvTK0TWVl7SIUy/3czjWjDg2XNX1MEQ0O48KeyV0Uuui62jh1g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SEYPR06MB5515
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 14 Sep 2023 05:04:41 -0700 (PDT)
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email


在 2023/8/15 4:51, Jonathan Corbet 写道:
>>   /**
>>    * dump_tasks - dump current memory state of all system tasks
>>    * @oc: pointer to struct oom_control
>> @@ -484,8 +513,8 @@ static void dump_oom_summary(struct oom_control *oc, struct task_struct *victim)
>>   
>>   static void dump_header(struct oom_control *oc, struct task_struct *p)
>>   {
>> -	pr_warn("%s invoked oom-killer: gfp_mask=%#x(%pGg), order=%d, oom_score_adj=%hd\n",
>> -		current->comm, oc->gfp_mask, &oc->gfp_mask, oc->order,
>> +	pr_warn("%s invoked oom-killer: gfp_mask=%#x(%pGg), order=%d, policy_name=%s, oom_score_adj=%hd\n",
>> +		current->comm, oc->gfp_mask, &oc->gfp_mask, oc->order, oc->policy_name,
> ...and if the policy name is unterminated, this print will run off the
> end of the structure.
> 
> Am I missing something here?
Perhaps it is inaccurate to use policy name in this way. For example, 
some one use BPF_PROG(bpf_oom_evaluate_task, ...) but do not set the 
policy name through bpf_set_policy_name. In this way, the result is 
still policy name=default, which ultimately leads to error print in the 
dump_header.
I think a better way:

+static const char *const policy_select[] = {
+    "OOM_DEFAULT";
+    "BPF_ABORT",
+    "BPF_NEXT",
+    "BPF_SELECT",
+};

struct oom_control {

  	/* Used to print the constraint info. */
  	enum oom_constraint constraint;
+
+	/* Used to report the policy select. */
+	int policy_select;
  };

static int oom_evaluate_task(struct task_struct *task, void *arg)
{
...

+	switch (bpf_oom_evaluate_task(task, oc)) {
+	case BPF_EVAL_ABORT:
+              oc->policy_select = BPF_EVAL_ABORT;
+		goto abort; /* abort search process */
+	case BPF_EVAL_NEXT:
+              oc->policy_select = BPF_EVAL_NEXT;
+		goto next; /* ignore the task */
+	case BPF_EVAL_SELECT:
+             oc->policy_select = BPF_EVAL_SELECT;
+		goto select; /* select the task */
+	default:
+		break; /* No BPF policy */
+	}

  static void dump_header(struct oom_control *oc, struct task_struct *p)
  {
-	pr_warn("%s invoked oom-killer: gfp_mask=%#x(%pGg), order=%d, 
oom_score_adj=%hd\n",
-		current->comm, oc->gfp_mask, &oc->gfp_mask, oc->order,
+	pr_warn("%s invoked oom-killer: gfp_mask=%#x(%pGg), order=%d, 
policy_name=%s, oom_score_adj=%hd\n",
+		current->comm, oc->gfp_mask, &oc->gfp_mask, oc->order, 
policy_select[oc->policy_select],
  			current->signal->oom_score_adj);


And all definitions of oc should be added
struct oom_control oc = {
      .select = NO_BPF_POLICY,
}

Delete set_oom_policy_name, it makes no sense for users to set policy 
names. :-)

Thanks
Bixuan Cui