Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <5343d12a-630c-4d54-91f1-7a7d08326840@vivo.com>
Date:   Thu, 14 Sep 2023 20:02:59 +0800
User-Agent: Mozilla Thunderbird
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>
From:   Bixuan Cui <cuibixuan@vivo.com>
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?NDNRb3J4TWV6NFpBcERScGlRRGgrcThYdFFmbTVKdk5zNklFQloybllSbGVR?=
 =?utf-8?B?bGMvMXJHeEE1ZFE5cHhYMm4rRjRxdCtITlFzZDFTUjFRR0txY2UyZHZRQmhL?=
 =?utf-8?B?VCtZZFJKSjRwdTF5YUhKT1Y0Smo3STg4UFpxTk05QWxTZFRjOTY4UlRRbjd0?=
 =?utf-8?B?UTN5SVpmbDVlM2RZWHFzVDZoeUpVWnAzVDFuU0xvMktKVFl1ak1zTlVWYjB2?=
 =?utf-8?B?THlDSmI2UFNWRGJjWFVOTWRzWXN4OTUwN01XVWEvOHk5clpha2JERVE3YmRD?=
 =?utf-8?B?eGhXQXdZK1dtZ1VRMHB3TUo3OEpXczIzc2hXdkRiREViMFZHR09DcEVWQVNE?=
 =?utf-8?B?a3RycUd4QUhGM2VOblhtK1RPQ2prWG1JQkRoSFJkNlY1VTVydmJIUjg5OUFP?=
 =?utf-8?B?SzRkQWY1ZG9jUDNxVzFWUVZMNnJQWjlkQTJmcjA3ZXVlY2J0WUp4dG5Fbldu?=
 =?utf-8?B?MXZtdjIzS0VXN0s5L25mWlZEKzZzeW9KYlh1WWRrbnhMVXdrRTIzOURhazJO?=
 =?utf-8?B?MTJuNnNOc2NnbVpQem15ZDhSMlVFcW9jMmVKcWJNMkNtRzRtUkdsSlordEpx?=
 =?utf-8?B?Zk43Y1ZXUkpDNkdHQU5YRXpRSWdyanpRbUhRVEtKQXN0Ny8wYVBMLzFGajlw?=
 =?utf-8?B?V1pqeklGRFBvdE0veVBxOGMzTzIxMEtpdnZmNmRFUEJlaE9KRkFLNytEWTR4?=
 =?utf-8?B?TC8vUXJWUHFTMFZ4bTQweXAvc2pQMlFFZ0VUVzk5Uk9HYWNqNkJxdXFUUmFX?=
 =?utf-8?B?dm9hNzZwMlJGOXM4ZCtzcWdqd1RFVEJ2U2YyMzF2eDdEaDAvNVRGSm96WlZK?=
 =?utf-8?B?M1J4N3B3WS94VEx1ZkdKQkFzMmp4TU1kckIyZkpYODlRUFluQys3bzFsVTVi?=
 =?utf-8?B?eE1UZGhEcmtBUERnUjF0S1hIb0I4T0dhQ29Ra2NHK3lxeDVQMFJKRklGTDFH?=
 =?utf-8?B?dTViWnkvaGxYaE16Q2xIczZZOS9iTnVVYUhxQXBaazBudS9odUJIamp5M28y?=
 =?utf-8?B?QkVVWDNpMTBNdTVlWitxVWwvYVFSblFwK3Vtdmd6NTNLbGlvdFhaUVBtdC9x?=
 =?utf-8?B?dm83MkVFdVVsU0pTZmcrQnNGNVpaQ0JGcVFMcW9WeTFsZWxhWnFjdDh0TzlC?=
 =?utf-8?B?STJDMWVYVkhvWkRQdzRVRTJxdUFPRnZjeE1vdGMyWDFaMDlTUGtCTnFFbU40?=
 =?utf-8?B?UG10WUlocWxqaWdMRGdkOE5Vam9hNW5jc0xXcEZjcHk3Z2ZyS25yS0trTVdY?=
 =?utf-8?B?c2tNV0tGb0pReVJ5bWorS0dJNG1vVTAySDROdi9yMlBSejVVY3M4UkZOQW1k?=
 =?utf-8?B?c2x5Rmh1eG5XYjFUYUFQNmNoa2Q5bUl5V09NR0FCRGcwN1pNZXNRSmczdDRI?=
 =?utf-8?B?RlRjYldEOHo5dXpVcmpaTEFQVVhPYitTbGlxcy9zL1dkTXN5RFdZKzV4U3hU?=
 =?utf-8?B?bXBpTkFKRG9Wb2k0Q3ZmemlLaTJLSHp6SWIxUTlXZFBYb3FYSU1abmZkTndM?=
 =?utf-8?B?WWQ1YWJOMDYyc2l6UkFnSEZXcmVLcDRGSGZFWEdiMEtPbldWZ0V5LzJrTXhv?=
 =?utf-8?B?cEJyMmZYbmZ3MjJQWXpRRGlyK0Rrd0gwTElXMk9UazNKZWRpT3Q1VjZMNDNv?=
 =?utf-8?B?M1JQdUtncDJQcW4zZzFHZjh0Szh4cHhmZTIzaklDTHI3Q2ZTbHpNb21NcHd4?=
 =?utf-8?B?Q0x0aklxb3gwc29XUHNYeFRJdkpNYjN0WW15cGc2NnJUUlVVdDJJL2RWanJs?=
 =?utf-8?B?ZENvQUQ0SVE0cUFjditYc01HSXpwWXFZZUVYZk1vd0tEWUI5VkdnNzA3L0FU?=
 =?utf-8?B?MzVVbGZGd1E2LzJ4dUpORmx0TnF3Zm9WdUVjZExFNW9lT3hpK1FRSTB6cnVm?=
 =?utf-8?B?em51UGx3YWJpdHo4VjRZTTZKWEE0d0Q2bm9LMzcraE5USjZSMWNGTkxmcUFk?=
 =?utf-8?B?UEpxQTRCcHF3NXQ2TWhZNTFiSE9rclhlOWtmUHlkMzU5QWZENzNtQ3NzS3Yr?=
 =?utf-8?B?RTdJQk5yR3VxYTVYNVFPanMvcHl5V1J3RzlYK0sxYVBReHBYd3M0RUJKdXRy?=
 =?utf-8?B?eEpnUngxUEVxMkVMNUIyN05ja1ZIRE5NcFZBTzV3RjlTSnJTK1lBVE0wbnVC?=
 =?utf-8?Q?HxGo03C8ShlkQRDcGq6PZwqVp?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 82a7301b-d647-4e5d-06dd-08dbb51a8d74
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:03:05.1465
 (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: vLFXKE5bwYgb6Cmn/mTF0P38L23ncpiD0XTXdqHS9EINSi7eYCAySw4U6HqO6e0lbClSk5KOhhIubZ9xG8PIZQ==
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 (snail.vger.email [0.0.0.0]); Thu, 14 Sep 2023 05:03:12 -0700 (PDT)


在 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