Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <d07546ea-0dd3-f795-ee04-e4b40db5354d@oracle.com>
Date:   Mon, 14 Feb 2022 23:19:38 +1100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.6.0
Subject: Re: [PATCH v5 1/2] kernfs: use hashed mutex and spinlock in place of
 global ones.
Content-Language: en-US
To:     Tejun Heo <tj@kernel.org>
Cc:     gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org
References: <20220206010925.1033990-1-imran.f.khan@oracle.com>
 <20220206010925.1033990-2-imran.f.khan@oracle.com>
 <YgKm5aSCcCYWkck2@slm.duckdns.org>
From:   Imran Khan <imran.f.khan@oracle.com>
In-Reply-To: <YgKm5aSCcCYWkck2@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?OFROMUtaS08yZFNScnN0Y0VQdEFxZkxuOVpidnpwQkZGcXdaOHVrVUFGa1BH?=
 =?utf-8?B?NVpEN0dnQmthaXAwNFJ4MU1YZ2FBTXk5bUNiUldRbWwySlZUSUYxS1BJTzhH?=
 =?utf-8?B?MVFOVWxZcks4b3hqRlRjM3V0d0x4aXd5WXZ1bURFRG1oQUdvd1diVFFUV0hL?=
 =?utf-8?B?NjFPeVVSdm1IcCtydnlGQ2Z4OGZIa21pWS9VWTRPTDZiWklZOFBOSjR3Znp1?=
 =?utf-8?B?SlFsbTVpbUdDdDBIUWFveHFubk5pNjU5T2hMUURRTU9iUFZ2R1ZuVjVsaVc5?=
 =?utf-8?B?dE5TSEoranBHVVdOQnNIMHYvSG1EM3krRWVsVHVGUkdVOUkrbkE0KytsTE1E?=
 =?utf-8?B?dG1WZUZMRG5DSW1ZV1Y0N05mSklBUTg0MXVodTdZak0zUzdVeW00Y1JxRVZ1?=
 =?utf-8?B?bFpmYWI4U1JFa1pXQ0dyNWxBZVBNaVhTNm4xVjlHNHhERndUNEl2RHYrb29k?=
 =?utf-8?B?T08xOFZ4NWdVOTAvbERrNlRzWEd3dm5BUFY1NGFKc0plUFVmbnQraXRKK1NN?=
 =?utf-8?B?Vmp2bGRpSGxkamRKVXJEREErQWNkQXozT0lnR2dhMnVML2pYTHpBT3JnMXZ2?=
 =?utf-8?B?RnhmM3VSclc0YXFMQmlSRnRGaE9PSEhlRk11cmhxdkc5dmc5TDF5YkRGUXB1?=
 =?utf-8?B?MXJtUzUxK2pGaXltRHk0dmVnTGhQUkxjMFJJdDg4bFpBSHR5SGZjalJSV24x?=
 =?utf-8?B?eVZYaHpSUXhpRTFHYS9UdmRkcWpTcHBLU0Y2UXoyYi9oNWR6bXBiSGRaN25V?=
 =?utf-8?B?eEV5VnhFWDF1OFFSMUJCNHo0cGltZFpMZHhuc0ZRTnZLTWE4NEpkaUNVTGt2?=
 =?utf-8?B?RE9qYUMwRUZGZzFDZjdWWHcralEzQ3F6SjllWGZIWm5YWTc2dUtNVGtNeThh?=
 =?utf-8?B?T2x0V0VhbWdLcjlpMCsyYzZiSTBrN0FJY1BvaFc4NTR3WklHKytQOGRlVU5M?=
 =?utf-8?B?UTduekpjeEtKaFJMRk9qRXE3dzFzK0FEV1h0ZmFPdHd3eVN6bHVTdHhqbkdK?=
 =?utf-8?B?UmRmdVRoY2ZyS0puT3J3Y3F6eVVHSkF3RkRqM2JWdGdSdFZmZ2Zqc3pVYkV2?=
 =?utf-8?B?WnE1QkVERGpjTlJGelJPREVVOEt3c2FUbXFJejN4WWVYNzJWYzdoelhqOE5w?=
 =?utf-8?B?REJzckp2ZEdvOXNGUStmZUo5SDdXcHc3T3lQN2pWNG9TMWdZZUp3NStWaFY1?=
 =?utf-8?B?Y1VNeWE1eG1EZk5SNjhreEZWVDNkbjU3cWtwT0d3dVZqbWY1Y3Y1dWVQeGda?=
 =?utf-8?B?TFE0L2dWNER1bGlncjlSREpIYjlTS2xpWEtxY2k5cGwzWWlzMGF5RzZ4MTdi?=
 =?utf-8?B?MVRxYUxlbEswYjlYdmU0SzJxY0pKRTRTQkpiOW1URFJaTnRzdnBULzdEb2tr?=
 =?utf-8?B?a21udUk3TG1hTmRobUJCTktFZ0ZoMW9MdGcvSFk5am5UdGVZYmVtNE5TZ05Z?=
 =?utf-8?B?VVoyejdtam10cHB3ems4SUlBMWNTSW0vZ1Z1dWIzbnJoaCtFSWhjOEQvRGtY?=
 =?utf-8?B?YU1pOGxtelRUaEhoQnpCNmluaVVIeDJQeGJHWStsSUpnNStHTUt4dUVIU2wx?=
 =?utf-8?B?b2NaYndXTUlxMEk1cndjeENSNndHMkdRc09CbGZPMzZpUk14QmIzMmxXY09R?=
 =?utf-8?B?Y3c0YVdnS1dwZUozVFllZmJad3k4NVUyWitsdDRXaGZ1ZDJPTnRtaHVncXlp?=
 =?utf-8?B?YnhRTTZ1bUI5OGtKeDkvcG9RTDVZbm9weXhQMWs4KzMwamh2cDNzK3gwUnBJ?=
 =?utf-8?B?RjZmNjJBZ1JaTThVZGN5cndUTU5kc2daRm1ISGNROUhDQXFVaXZFNHJsUDVm?=
 =?utf-8?B?RUgxZlFjWjJXQnRDT29YMWEyclFwMUdrTUgwVlprbG5zNGlqbHVvd1pGQ0gx?=
 =?utf-8?B?SHNtV3prVnJYdEthdDBJWExuV2hsN1VtT0RVMnQrWENMY3F4V0xKVlZlRDNn?=
 =?utf-8?B?WTJZWHJ4RExSNll3QUJUeXhQeFN0c2VyS0VvRU5OUDk0dGpxOXoreDFDVmFN?=
 =?utf-8?B?ZzZ6OWZPeXI1b3AySEF1c3AxSTZ4NkI3WVFLNUw3b21JVVp0M28vS1BFbGpB?=
 =?utf-8?B?R241M0Uva3FjaHNDMCtiOWppTFVLYnIwN1J2MXUvNllYWGtHc1ZjdmwwWjBT?=
 =?utf-8?B?YVhZRGFZeTFUTVpqeEhrcTF6cE9jNlk3cGk0TXdadDlCejRaSFI3bWpzd2tX?=
 =?utf-8?Q?I6eA/o6SFwjSZkDCdQ17n4c=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d23be297-4f85-4e6b-553c-08d9efb44aa8
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4468.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2022 12:19:47.8301
 (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: tlrJTQucO3tMvmR2FQDFSy1aNpTbCCgzLxznqjI5CB7gfscGwWJUX4umOriAfUo7xDvPRIxq02OzsjL4jl21Wg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR10MB1827
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10257 signatures=673431
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 spamscore=0
 mlxlogscore=999 phishscore=0 bulkscore=0 malwarescore=0 adultscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2201110000 definitions=main-2202140075
X-Proofpoint-ORIG-GUID: PfkPfzvQBinZbGKktLSFpOlwuN1orqRV
X-Proofpoint-GUID: PfkPfzvQBinZbGKktLSFpOlwuN1orqRV
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hello Tejun,

Thanks for your feedback.

On 9/2/22 4:22 am, Tejun Heo wrote:
> On Sun, Feb 06, 2022 at 12:09:24PM +1100, Imran Khan wrote:
>> +/*
>> + * NR_KERNFS_LOCK_BITS determines size (NR_KERNFS_LOCKS) of hash
>> + * table of locks.
>> + * Having a small hash table would impact scalability, since
>> + * more and more kernfs_node objects will end up using same lock
>> + * and having a very large hash table would waste memory.
>> + *
>> + * At the moment size of hash table of locks is being set based on
>> + * the number of CPUs as follows:
>> + *
>> + * NR_CPU      NR_KERNFS_LOCK_BITS      NR_KERNFS_LOCKS
>> + *   1                  1                       2
>> + *  2-3                 2                       4
>> + *  4-7                 4                       16
>> + *  8-15                6                       64
>> + *  16-31               8                       256
>> + *  32 and more         10                      1024
>> + */
>> +#ifdef CONFIG_SMP
>> +#define NR_KERNFS_LOCK_BITS (2 * (ilog2(NR_CPUS < 32 ? NR_CPUS : 32)))
>> +#else
>> +#define NR_KERNFS_LOCK_BITS     1
>> +#endif
>> +
>> +#define NR_KERNFS_LOCKS     (1 << NR_KERNFS_LOCK_BITS)
> 
> I have a couple questions:
> 
> * How did you come up with the above numbers? Are they based on some
>   experimentation? It'd be nice if the comment explains why the numbers are
>   like that.
> 

I did some testing using different number of CPUs with qemu and the
above numbers were collected from there. It may not be optimum but this
is what I could think of while doing those internal tests.

Do you think it may be better to make this configurable via Kconfig.

> * IIRC, we split these locks to per kernfs instance recently as a way to
>   mitigate lock contention occurring across kernfs instances. I don't think
>   it's beneficial to keep these hashed locks separate. It'd be both simpler
>   and less contended to double one shared hashtable than splitting the table
>   into two separate half sized ones. So, maybe switch to global hashtables
>   and increase the size?

I have removed the hash table from kernfs_root. Doubling the size was
not giving any improvements so I have kept the hash table size same as
before. This change is present in v6 of patch at [1]

Thanks
 -- Imran

[1]:
https://lore.kernel.org/lkml/20220214120322.2402628-1-imran.f.khan@oracle.com/


> 
> Thanks.
>