lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <132f7a1a-3219-bdac-5ca4-ad5a54b09616@huawei.com>
Date:   Mon, 21 Aug 2023 14:35:30 +0800
From:   Tong Tiangen <tongtiangen@...wei.com>
To:     Matthew Wilcox <willy@...radead.org>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Naoya Horiguchi <naoya.horiguchi@....com>,
        Miaohe Lin <linmiaohe@...wei.com>,
        <wangkefeng.wang@...wei.com>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm: memory-failure: use rcu lock instead of tasklist_lock
 when collect_procs()



在 2023/8/21 12:34, Matthew Wilcox 写道:
> On Mon, Aug 21, 2023 at 10:25:34AM +0800, Tong Tiangen wrote:
>> +++ b/mm/memory-failure.c
>> @@ -546,24 +546,26 @@ static void kill_procs(struct list_head *to_kill, int forcekill, bool fail,
>>    * Find a dedicated thread which is supposed to handle SIGBUS(BUS_MCEERR_AO)
>>    * on behalf of the thread group. Return task_struct of the (first found)
>>    * dedicated thread if found, and return NULL otherwise.
>> - *
>> - * We already hold read_lock(&tasklist_lock) in the caller, so we don't
>> - * have to call rcu_read_lock/unlock() in this function.
>>    */
>>   static struct task_struct *find_early_kill_thread(struct task_struct *tsk)
>>   {
>>   	struct task_struct *t;
>>   
>> +	rcu_read_lock();
>>   	for_each_thread(tsk, t) {
>>   		if (t->flags & PF_MCE_PROCESS) {
>>   			if (t->flags & PF_MCE_EARLY)
>> -				return t;
>> +				goto found;
>>   		} else {
>>   			if (sysctl_memory_failure_early_kill)
>> -				return t;
>> +				goto found;
>>   		}
>>   	}
>> -	return NULL;
>> +
>> +	t = NULL;
>> +found:
>> +	rcu_read_unlock();
>> +	return t;
>>   }
> 
> I don't understand why you need to modify find_early_kill_thread() at
> all.  It's still true that the caller holds _a_ lock protecting it; the
> comment needs to be updated to reflect that it might be the RCU lock
> or the tasklist_lock (or did you change all callers?), but there's no
> need for this function to take the RCU lock itself, afaics?
> 
> .

I've checked that all the paths that call find_early_kill_thread() 
already hold the rcu lock, and there's really no need to hold the rcu 
lock here.
In the next patch version, here only the comments are modified.

Thanks,
Tong.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ