Message-ID: <3173d8cd-108e-54b1-e72d-19f32e6adbb7@oracle.com>
Date:   Fri, 3 Mar 2023 08:28:38 +1100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.8.0
Subject: Re: [PATCH 2/3] kernfs: Use a per-fs rwsem to protect per-fs list of
 kernfs_super_info.
Content-Language: en-US
To:     Matthew Wilcox <willy@infradead.org>
Cc:     tj@kernel.org, gregkh@linuxfoundation.org, viro@zeniv.linux.org.uk,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        joe.jin@oracle.com
References: <20230302043203.1695051-1-imran.f.khan@oracle.com>
 <20230302043203.1695051-3-imran.f.khan@oracle.com>
 <ZADJ85K6KTb6XiR4@casper.infradead.org>
From:   Imran Khan <imran.f.khan@oracle.com>
In-Reply-To: <ZADJ85K6KTb6XiR4@casper.infradead.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?V1N3R2tGaUMrUkdJbUk4Uno1WHJiMEcxQ21sVDNwNjUyWk9uWkFrT28rK0Er?=
 =?utf-8?B?b0Z3TC9LSHlZTHB4WEN4SXo4L3AwRjh3MklNMkhZK0lNOUFzKzNzM0ZYTWJu?=
 =?utf-8?B?c0tvc2MwZUtOZnJDaEx1K1pGRmFuTHNub0VLRHFoKzE1SWM0N3NSSWtsTVpM?=
 =?utf-8?B?eUhSUDBNdkhoTnJ0cis2NDhVN3RwRFBFTmU4dHdiSG5vVm5DZTlJMkxUNXp5?=
 =?utf-8?B?S2lERC9iODNVUHI0dnZHcHNKa1lDQnFiZWxHWFhrajZtWFF6cWJPVjFwZ1ZG?=
 =?utf-8?B?eTVuSHNqNHd1aUdEUFpYRHA4UWs3YU96OXBnczVDSWZ0dGQ0TmVnY3BZaEQx?=
 =?utf-8?B?OWR1RGpiaG5Cd3pDVjNxRW9MVDk0Rit2S1J4bHZYZ3FmVGhtc1JCcHNJQW1t?=
 =?utf-8?B?c3pLbTBqdnFXQVNvWUptUmp5MnQxUGpUSlJhV2F0UVVwL0J3dUpGREtmQjZD?=
 =?utf-8?B?WDAzYWszK1E0ZWhvZmtkVTFwajdtQWNBUUtrOCtyVVpOZU5rRE9HYVRPQmNp?=
 =?utf-8?B?K2c1NFYycW9CbWNESzRrUUtKY0tLVk0rcE5tSWlzRjZlS2FQSlJGM0hjbTJr?=
 =?utf-8?B?c2JPWStqTWRjbUsreEFpdW82SVdNeUd3dXo2RnQrcTdPcUNqRENjUjJWNWhY?=
 =?utf-8?B?RGh4b2lBSGEyT0wrWnRzVEdzWTBRK3I0czJmZFJPYVJrQUpTUnNhaHVmK3VP?=
 =?utf-8?B?OWM0b0ZvMTdtOFlpaHFqUFpndmZxcmxmbjBqY1ZQcHFyVUN6V3dLSnhtZTdF?=
 =?utf-8?B?UGIyS2xIUjVEUmVzUjNkRGorZjVQVzJGL2tDa0cvTFRLM3BxWGZGb1N6Qy9h?=
 =?utf-8?B?MWJ5dVQ0TnBHRitoUUVTYUUyWG01WmRlRTNBcGZVQkJwbWNGQ0E3UFNMRVhr?=
 =?utf-8?B?cHhETUJ0TGFoMDBhM2h1YnZoZ2FvcnZSZGNISTJSRnFTeUFQZWc3M2ZpajJ0?=
 =?utf-8?B?RHFIK01WV0xwR3NUUXBMQUlDckxKRVBkVHNQVWlzb0U1Z3JCT3RoSDRZV1Jx?=
 =?utf-8?B?SStwb0l0RWxSUUtNT1YwTWx2Zk56TjRQVnhucUg4WFYvVEVjTklvNm5jS01o?=
 =?utf-8?B?Vzl4cnV5bmVkZW8wZ0xJNk10bWc0YkdlMEhOaWFaRUJ5bnNLZmNGRFFmQ3dU?=
 =?utf-8?B?RytjZTFCVFVqbTVVRC9uTDJOakg1MndPL2JoUUswUURVTjd6ZlA2TitoaGlq?=
 =?utf-8?B?OFp3R2xRbTVTVi8rN3o0eFd6YUplcjFWdjdZNEMycXBkanc2M0FCSkVxZjUv?=
 =?utf-8?B?UlVOcWg3NnVpdm1nZ0JaeWI2bHMzM2kyM1ZrcXA1dGdXRzBWSjdLTnQxRXNk?=
 =?utf-8?B?K1lGQ0g4Zkd2V1IxSU52N05ZVEVNb0VZQzB3WC9GcGQ4TXVOMjJ6LytQSEda?=
 =?utf-8?B?Q2UySDNoekF1M0dUcmhmd0psaEhQZXJ1VUh4SWJ4OU9zL3ZEenFmUTYrd3Ja?=
 =?utf-8?B?ckZVNnh1eTZXcGpJMy9pbnRmWFV5Y3Bycko3bFE1NFZZYUtMbXVnSFFVdmU0?=
 =?utf-8?B?V3NSTlRaUk5UclYwTm9ONStuTFRmSkREYjM1eUp4MjRjTWxGSURPa3F1LzNs?=
 =?utf-8?B?c0k4WVIxZHExMVVTaC9YZFIvTEZYaGttUE01MVBUQWZPa09UdUdXM3R4anY5?=
 =?utf-8?B?N1I0QzNJdCt5NGVqeHJtRXJIRWZoYzl4cklwNGNaVkNPUHNsVFMyV1BTTlha?=
 =?utf-8?B?aUpVQ0d6WjJkMWVUblpFRGNxbHoweEIvcGwvMGw4Y2RNUUdkdndsYWo2QmE4?=
 =?utf-8?B?ak1OdDM5dUNOYzVKTTJzUUJMTDJiWkhkYUVjUFZXMnJFZWUyR2piZmlYWGhQ?=
 =?utf-8?B?Z2hnKzRIR3NqSFBWOThrOCtoVHgrZzFNcmFNWGFWLy9yVlRNNUZWT2xvcVUr?=
 =?utf-8?B?c1pZdWF3WUdnZXVLTWFTckVUdlRLZW11b0lxZFJJSjZYLzJjaVNyTHprZDE1?=
 =?utf-8?B?RGp2TmN3YTljdVR1T1g3clNNNXM1TUo1aExZMFdKV0R6V0JWb2tacHpTMzVv?=
 =?utf-8?B?RVUwbHRhZ2p1aUQ1SXZjSUZVdHJCZHE3QmViTlNJMjV2dnFDNDRVb3VTd0ly?=
 =?utf-8?B?TVk2NkhIYXN2WSthWVpoR0syWUxsMEk0YXdBZm1GbWt1RnlLT0F2SkNNNmlj?=
 =?utf-8?B?NUEyeUpoWnA4V1k3N0xBY3htMGtzZTNpWnhDQ3RHTnBGeGlxSVNBV01ZdXJT?=
 =?utf-8?B?Z3c9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 1LOsalwUmIGaJ9ug1yaMSvcHIctXXSh4kTJ2ZdBhHJRs4MbqjEDRImPKVlIcP+AIbCHvP41PPXvcFbWCpg9HYKjxdjK7Bubsuyi4qKF8oBbB5yVlG4S5xFyYZTcubbojR/TMIgffED2ojsqm/OmMFQIXJ6rix2nx2HoGQxwTkvpWNczUmt9IZh7+1z+LYMn0GGfuYeYi1Cq5EufdSTfs1XkyylKxFOTBtjWHU3DX8QdNNL0Gyn099x2jLX18MnA24LK6FIJuwfi39NTXXnEU0DP+R+g/eLJosHismmjwxCDFJz/PWtyAuZ01l7MJZVVA7deVpzhCKy8uMpEYVaOYWaPfxLS5YpkRW2UpPk9xVWmpx3l4as+dwIY2gTblJ6ddb13ra4acBcqsrMS1w1riEqxOG2KwDoHI3LkeYgZfKvnvxZHg99QVeGb7X4YwusNlqxxjuU6qb9jrTRImlRTJvdW9N/1CzmhufVcH8pmTXBQSlQTgbFAZwf5h+aPwCoDRv9V/ymwQ1TOwY7RE7p1hKyuIpj9sTpnQfz27Ldl6syaTa5FxHetvEYTo2ptcEnSlk7D6OFMazqK4wm0ViDSY5YUbBb+BXw3JrVBN6J+WxpIVVCPXZacFs0XxejzqT3uvsVTh7fPop1kU6vLOjmTpw6Rzg7AjJIwtpzwm4GQQoz9PVUw2YcbllmkqAl86D8OrBvsjHkoD75T9uRKx9z7iemYCC71gYwWDXVX6Al6I/KyVlJtck6qQ1XDk4KJbqaSN5hQQJc02jdl6BGT+iN99zlNEtejxIj0lOae7pMG7VbqnYtSvsp96PK+aXznt/yFy/7kCP1qssTblv1nAdfcJbsR9UV1uJocXEfNOOnvhq3pXEvWmKKhiKW0Z7u6KdRB8pAuUCu5xee/HXTx5sjDiURs6Wk6pGCJEHmwkJG8AIGE=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fb30fe71-86d7-4b31-d67d-08db1b651b86
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4468.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Mar 2023 21:28:47.3493
 (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: 0aX0yxWwUhTb1mSWro73bMmOooD7+/UjTDfhHvKhv38P2mcMwXBKiUSS6PbiJTBdRqAf+QYEJcImmgZzoWiLWw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR10MB6788
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-03-02_15,2023-03-02_02,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 bulkscore=0 adultscore=0
 mlxlogscore=999 suspectscore=0 phishscore=0 spamscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000
 definitions=main-2303020184
X-Proofpoint-ORIG-GUID: FPukBjisUWKkJvD7iIcXs3vf3kCPUGrC
X-Proofpoint-GUID: FPukBjisUWKkJvD7iIcXs3vf3kCPUGrC
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hello Matthew,
Thanks for reviewing this.

On 3/3/2023 3:08 am, Matthew Wilcox wrote:
> On Thu, Mar 02, 2023 at 03:32:02PM +1100, Imran Khan wrote:
>> Right now per-fs kernfs_rwsem protects list of kernfs_super_info instances
>> for a kernfs_root. Since kernfs_rwsem is used to synchronize several other
>> operations across kernfs and since most of these operations don't impact
>> kernfs_super_info, we can use a separate per-fs rwsem to synchronize access
>> to list of kernfs_super_info.
>> This helps in reducing contention around kernfs_rwsem and also allows
>> operations that change/access list of kernfs_super_info to proceed without
>> contending for kernfs_rwsem.
>>
>> Signed-off-by: Imran Khan <imran.f.khan@oracle.com>
> 
> But you don't remove the acquisition of kernfs_rwsem in
> kernfs_notify_workfn(), so I don't see how this helps?
> 
Yes. kernfs_notify_workfn should no longer need kernfs_rwsem. I will fix it .
> Also, every use of this rwsem is as a writer, so it could/should be a
> plain mutex, no?  Or should you be acquiring it for read in
> kernfs_notify_workfn()?

Although currently kernfs_notify_workfn acquires kernfs_rwsem for writing, I
think even w/o this change acquiring kernfs_rwsem for reading would be enough
since we are not making any changes to kernfs_super_info list.
Based on this logic, I think taking iattr rwsem for reading is right approach.

Thanks,
Imran