linux-kernel - Re: [PATCH v3 2/2] kernfs: Reduce contention around global per-fs kernfs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <84861be0-519a-233d-9c6c-482f12e33328@oracle.com>
Date:   Sat, 15 Jan 2022 04:08:10 +1100
From:   Imran Khan <imran.f.khan@...cle.com>
To:     Tejun Heo <tj@...nel.org>
Cc:     gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] kernfs: Reduce contention around global per-fs
 kernfs_rwsem.

Hi Tejun,

Thanks for the review.

On 14/1/22 3:42 am, Tejun Heo wrote:
> Hello,
> 
> On Thu, Jan 13, 2022 at 09:42:59PM +1100, Imran Khan wrote:
>> @@ -748,11 +749,14 @@ int kernfs_add_one(struct kernfs_node *kn)
>>  		goto out_unlock;
>>  
>>  	/* Update timestamps on the parent */
>> +	rwsem = iattr_rwsem_ptr(parent);
>> +	down_write(rwsem);
>>  	ps_iattr = parent->iattr;
>>  	if (ps_iattr) {
>>  		ktime_get_real_ts64(&ps_iattr->ia_ctime);
>>  		ps_iattr->ia_mtime = ps_iattr->ia_ctime;
>>  	}
>> +	up_write(rwsem);
>>  
>>  	up_write(&root->kernfs_rwsem);
> 
> Hmmm, so the additions / removals are still fs-global lock protected. Would
> it be possible to synchronize them through hashed locks too? We can provide
> double locking helpers - look up locks for both parent and child and if
> different lock in the defined order (parent first most likely) and record
> what happened in a token so that it can be undone later.
> 
I have made changes inline with your suggestion to synchronize
addition/removal through hashed locks but so far I am not using tokens.
I am currently testing these changes (so far no issues seen). Before
floating next version for review I wanted to understand the reason
behind need of tokens. Could you please elaborate a bit about what needs
/ may have to be recorded in tokens. Just one example will do. It would
help me consolidate the next version of this change without overlooking
something.

Thanks
 -- Imran