Re: [PATCH] mm/oom_kill: simplify remaining RCU sections with guard(rcu)

[Ye Liu]

在 2026/8/13 16:49, Michal Hocko 写道:
> On Thu 13-08-26 11:26:33, Ye Liu wrote:
>> From: Ye Liu <liuye@xxxxxxxxxx>
>>
>> Replace the remaining manual rcu_read_lock()/rcu_read_unlock() pairs
>> in oom_cpuset_eligible(), select_bad_process(), dump_tasks(),
>> task_will_free_mem(), and __oom_kill_process() with guard(rcu)() for
>> consistency and simpler control flow.
> 
> Could you use scoped_guard instead? While all these functions do not
> perform anything pas the for loop this might change in the future and I
> find it more obvious what the actual scope for the RCU is rather than
> the full function one.
>>
Thanks for the suggestion, I've updated to V2.
>> Signed-off-by: Ye Liu <liuye@xxxxxxxxxx>
>> ---
>>  mm/oom_kill.c | 15 +++++----------
>>  1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 4b5c70aaece4..8bbb5ca00bb6 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -94,7 +94,7 @@ static bool oom_cpuset_eligible(struct task_struct *start,
>>  	bool ret = false;
>>  	const nodemask_t *mask = oc->nodemask;
>>  
>> -	rcu_read_lock();
>> +	guard(rcu)();
>>  	for_each_thread(start, tsk) {
>>  		if (mask) {
>>  			/*
>> @@ -114,7 +114,6 @@ static bool oom_cpuset_eligible(struct task_struct *start,
>>  		if (ret)
>>  			break;
>>  	}
>> -	rcu_read_unlock();
>>  
>>  	return ret;
>>  }
>> @@ -368,11 +367,10 @@ static void select_bad_process(struct oom_control *oc)
>>  	else {
>>  		struct task_struct *p;
>>  
>> -		rcu_read_lock();
>> +		guard(rcu)();
>>  		for_each_process(p)
>>  			if (oom_evaluate_task(p, oc))
>>  				break;
>> -		rcu_read_unlock();
>>  	}
>>  }
>>  
>> @@ -430,14 +428,13 @@ static void dump_tasks(struct oom_control *oc)
>>  		struct task_struct *p;
>>  		int i = 0;
>>  
>> -		rcu_read_lock();
>> +		guard(rcu)();
>>  		for_each_process(p) {
>>  			/* Avoid potential softlockup warning */
>>  			if ((++i & 1023) == 0)
>>  				touch_softlockup_watchdog();
>>  			dump_task(p, oc);
>>  		}
>> -		rcu_read_unlock();
>>  	}
>>  }
>>  
>> @@ -894,7 +891,7 @@ static bool task_will_free_mem(struct task_struct *task)
>>  	 * are dying as well to make sure that a) nobody pins its mm and
>>  	 * b) the task is also reapable by the oom reaper.
>>  	 */
>> -	rcu_read_lock();
>> +	guard(rcu)();
>>  	for_each_process(p) {
>>  		if (!process_shares_mm(p, mm))
>>  			continue;
>> @@ -904,7 +901,6 @@ static bool task_will_free_mem(struct task_struct *task)
>>  		if (!ret)
>>  			break;
>>  	}
>> -	rcu_read_unlock();
>>  
>>  	return ret;
>>  }
>> @@ -960,7 +956,7 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
>>  	 * That thread will now get access to memory reserves since it has a
>>  	 * pending fatal signal.
>>  	 */
>> -	rcu_read_lock();
>> +	guard(rcu)();
>>  	for_each_process(p) {
>>  		if (!process_shares_mm(p, mm))
>>  			continue;
>> @@ -982,7 +978,6 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
>>  			continue;
>>  		do_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_TGID);
>>  	}
>> -	rcu_read_unlock();
>>  
>>  	if (can_oom_reap)
>>  		queue_oom_reaper(victim);
>> -- 
>> 2.25.1
> 

-- 
Thanks,
Ye Liu