Re: [RFC PATCH v2 1/2] kernfs: use kernfs_node specific mutex and spinlock.

[Imran Khan <imran.f.khan@oracle.com>]

On 3/1/22 8:54 pm, Greg KH wrote:
> On Mon, Jan 03, 2022 at 07:45:43PM +1100, Imran Khan wrote:
>> diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
>> index 861c4f0f8a29..5ed4c9ed39af 100644
>> --- a/include/linux/kernfs.h
>> +++ b/include/linux/kernfs.h
>> @@ -164,6 +164,8 @@ struct kernfs_node {
>>  	unsigned short		flags;
>>  	umode_t			mode;
>>  	struct kernfs_iattrs	*iattr;
>> +	spinlock_t kernfs_open_node_lock;
>> +	struct mutex kernfs_open_file_mutex;
> 
> Did you just blow up the memory requirements of a system with lots of
> kobjects created?
>> We used to be able to support tens of thousands of scsi devices in a
> 32bit kernel, with this change, what is the memory difference that just
> happened?
> 
Indeed, this patch increases kernfs_node size by 36 bytes ( 28 bytes for
mutex + 4 bytes for spinlock). From current kernfs_node size of 128
bytes, this will be a ~25% increase in kobjects memory consumption.
I can replace the mutex object with a pointer to it, to bring down
the overall increase in size. Will the size change be acceptable then?

> There is a tradeoff of memory usage and runtime contention that has to
> be made here, and this might be pushing it in the wrong direction for
> a lot of systems.
> 
Agree. Could you please suggest if this should be made configurable via
kconfig ? I understand that this would result in 2 versions of some
functions but it will allow systems with large memories to avoid kernfs
contention.  We are seeing the launch time of some DB workloads
adversely getting affected with this contention.

Also based on recent movement of kernfs_rwsem into kernfs_root, do you
think that the above mentioned mutex and spinlock can be moved to
kernfs_root as well. Although that change would not help in my current
case, but it could avoid similar contentions between different users of
kernfs like cgroup and sysfs

Thanks.
 -- Imran

> thanks,
> 
> greg k-h