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

From: Imran Khan <imran.f.khan@oracle.com>
To: Tejun Heo <tj@kernel.org>
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/2] kernfs: Reduce contention around global per-fs kernfs_rwsem.
Date: Sat, 15 Jan 2022 04:08:10 +1100	[thread overview]
Message-ID: <84861be0-519a-233d-9c6c-482f12e33328@oracle.com> (raw)
In-Reply-To: <YeBWWWa030xTftWe@slm.duckdns.org>

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