Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <84861be0-519a-233d-9c6c-482f12e33328@oracle.com>
Date:   Sat, 15 Jan 2022 04:08:10 +1100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.4.1
Subject: Re: [PATCH v3 2/2] kernfs: Reduce contention around global per-fs
 kernfs_rwsem.
Content-Language: en-US
To:     Tejun Heo <tj@kernel.org>
Cc:     gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org
References: <20220113104259.1584491-1-imran.f.khan@oracle.com>
 <20220113104259.1584491-3-imran.f.khan@oracle.com>
 <YeBWWWa030xTftWe@slm.duckdns.org>
From:   Imran Khan <imran.f.khan@oracle.com>
In-Reply-To: <YeBWWWa030xTftWe@slm.duckdns.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZW9YZ0NiNnZvSTdyZlZ3ZGJxekh3VGRleTQvekQ2bDQxVCsyUUVOL2FjL3Qr?=
 =?utf-8?B?QWM0azNnelBTdi8yR1AxWWU4UTk3c3RNNlNwQnlGTk4zYWUyc09McFBUcUFO?=
 =?utf-8?B?eWJmdC9JSzBYbUZVc01VLzQ5bkFmWStpNWp6Vm9sSmk2bDlaK0RoUnQwZHVR?=
 =?utf-8?B?dmVoR0tGUXpNa29XUjQwekRmUHJlbThYVHVFVE9vL2dNcWRvdFN3MGV0T3JH?=
 =?utf-8?B?bEttUG4rTlBGcWp0NUZvYkRNOGFXa2dOQzlFZmRrSTlnQTVkcktsQ3d0Nnpp?=
 =?utf-8?B?T3daTjMwNTNTUGhXMGhvMENUdkdsSVlIWHcxYWtqMy9GeTBqanBGVkc4WDlr?=
 =?utf-8?B?bjlXYTRNZ3c2clYrMThoN2phaHFvZmhHYmxQWVJMNUxsZ09yRjl2c1Erb0Rj?=
 =?utf-8?B?TFFnNUVLN1dUOFl5bDZMRGp0Rm9UcGVBeEMzZDl4MDQwcVowalNGYUNPbFVR?=
 =?utf-8?B?b3lsVy8zc09ydi85cmIydml2S0RnL1pDM0V2N1M3dWdQVUY5S3Y5RmFSRFpS?=
 =?utf-8?B?U2wyb0QxcC80OGRCS0FzWHZzNjR2VjZoUDlhMXJZYlN4YnV4U3VUVHBwY3E5?=
 =?utf-8?B?YUx3NHdPOXNXNm8xa2xpVEJWNnkwQkp1clhCZlhzQ0pjaWw1cXJrNE9FbVVw?=
 =?utf-8?B?SkU2cTBqVVJ2WndYdzJXSzdPVG5KWFduR1BFeFZpcDFkYzJuN04zYzJVRnNK?=
 =?utf-8?B?R0lNZzdycnZVb3ZiYmlWK3dTL05rL3ozUDlsTjh6djBuWWdsQWFmREpkbzQr?=
 =?utf-8?B?WGF6anYwVkVQZWc4WGd0aG1ZK1VwVkw4b1FKMmgvUVl2TGZVRlQwVnNUMk9s?=
 =?utf-8?B?Vi9tS1RUajM5RlZ1U3psRlQyejdDakJHQVROYjNiMkNVcjNJZk1ITVNyOTRU?=
 =?utf-8?B?dzdDY3FRcGxFZHo5Tk0vL2V1MG5VV0tPRENJRjFIZXUrOGlRRmoyRUpmVEZY?=
 =?utf-8?B?RkxLS0JHdmNab3JCUU1PQ2RmSm03SkZKTHZNZkNYNFZwZjZwRTJOYk54S0FQ?=
 =?utf-8?B?ZlJhTllWMHJNZXR2d0tUQ1U0WC9iUWtHaWJWdjV3QlZMVklUTU5Sd2NZWllS?=
 =?utf-8?B?aklUTVd1RXhwKzdxck5VaXZ6UGdnN2d4c0lINFlvOU1KMlY2SXhyZTd0MEc3?=
 =?utf-8?B?WTZHVVN4QjRRb01GeTkxbGZhY29MN054NS9CK0lrdzFhQ05PNWdLQnhjd0NG?=
 =?utf-8?B?WTFCajk2cU1nSDJick1BMWxCSU1nVXlTZ0JSZmNlM0pqR0g3ZUJJTm00VXB5?=
 =?utf-8?B?T3VSMUFiTURVUGFTaGxNeXFHRUwyNURpWnpFeVZ6aXNKQXhxNWp2SGVIaldx?=
 =?utf-8?B?Z3BLUU80M1dHOXRQdVFnK0JTME1YWTJuZEVXTTR1eU1FUmlNWFA3K1dpa0Zk?=
 =?utf-8?B?YjBqbDJ6TXBZek1FNUtJU1QveW9ZTURmU2NIMVRkLzNjdE9UU3I4UXlhZWMv?=
 =?utf-8?B?WDIwNE5YN2VxUkxQYU5CT0hGVVRFUmVJNS9tZzBBazVUZjUvNmFpcmxqMFIv?=
 =?utf-8?B?cmRxMU90YUFmZFRESVZhbVhoZmtZclNRWGNNS1c1UUNpd1BmVytPVnB6RWwv?=
 =?utf-8?B?aFNZTzg4cTMrOW05U2pzZGFmVHowQzJHd2d5NzFWWFFhSGRjekRPTGpUcUR1?=
 =?utf-8?B?dWs2amI2Vlg3L0JCczJRRmlTWmtJNXZQOEtMSTR1SWRSZWlZTjFSb3Z2NmI0?=
 =?utf-8?B?QnMrTC92SllvSlY2QWorOTR2TW9XZHgvS3YxU0Q1L1VqWDg1TUVyQnZMbkZL?=
 =?utf-8?B?NXQ3TndEUkdvRU9YSFZGMTJqRWp4MDNVbmxBUktKT2cvNGFOYjNMTThCV09n?=
 =?utf-8?B?b2NhSVZFZDZPNzZYR2t1Y3Rpc3UyejZZL05zUUpwcjdmUHhzY0VUTkc2cVRL?=
 =?utf-8?B?dHBKR0owaDNKdkNXZmpPNVp0d3pGTGU4eExtUUthdUlPbHpraHZoQnp1ekZy?=
 =?utf-8?B?VG50bDlMM0poS1ZXZU1XbktONnpLSW5SMHdlc0IzYndGY053ZW9zV3lZa0ND?=
 =?utf-8?B?WjkxL3ZUOVpGV2YzcWd2OW5UTitBNHBJNmxELzF3cTJHWjBvaHd4U1hjK2cz?=
 =?utf-8?B?MzRBdkh4cXdUNUQ3RkY1QTZjS3FRL0hZUTBVaGpTbnJMRDFrUEtWekltRlRJ?=
 =?utf-8?B?NXN6dHNwMGlyeEFnNENFajhvejRGRzFieXRsSisrZGh2T3h5dVl1MEVEbjBQ?=
 =?utf-8?Q?t+yBIt/z4Y38JNZZnXdcJoY=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 817ee782-d316-417b-dd7e-08d9d7807636
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4468.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jan 2022 17:08:19.2165
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: U4aJYuCerPRuwWZxv4gbgV0oqKMjbQSGIREnLHxACosNQQQ5Hu2UeeJYNDG1hqyWlLMFI3yi+cqkFZpyQEo7SA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB3406
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10227 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 suspectscore=0 spamscore=0
 mlxlogscore=647 adultscore=0 phishscore=0 malwarescore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2201140109
X-Proofpoint-ORIG-GUID: RGpAeyydlHbku-eiMluwae_B9Gtwphup
X-Proofpoint-GUID: RGpAeyydlHbku-eiMluwae_B9Gtwphup
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.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