linux-kernel - Re: [PATCH v7 0/8] kernfs: Introduce interface to access global kernfs_open_file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e77bab80-c2ca-3ba4-862f-9a29268f132e@oracle.com>
Date:   Wed, 9 Mar 2022 15:12:01 +1100
From:   Imran Khan <imran.f.khan@...cle.com>
To:     tj@...nel.org, viro@...iv.linux.org.uk
Cc:     linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org
Subject: Re: [PATCH v7 0/8] kernfs: Introduce interface to access global
 kernfs_open_file_mutex.

Hello Al and Hello Tejun,

On 25/2/22 4:21 pm, Imran Khan wrote:
> Reduce contention around global locks used in kernfs.
> 
> The current kernfs design makes use of 3 global locks to synchronize
> various operations. There are a few other global locks as well but
> they are used for specific cases and hence don't cause severe contention.
> 
> The above mentioned 3 main global locks used in kernfs are:
> 
> 1. A global mutex, kernfs_open_file_mutex, to protect the list of
> kernfs_open_file instances correspondng to a sysfs attribute.
> 
> 2. A global spinlock, kernfs_open_node_lock, to protect
> kernfs_node->attr.open which points to kernfs_open_node instance
> corresponding to a kernfs_node.
> 
> 3. A global per-fs rw semaphore, root->kernfs_rwsem, to synchronize most
> of the other operations like adding, removing, renaming etc. of a file
> or directory.
> 
[...]
> 
> ------------------------------------------------------------------

Could you please review v7 of this change set and let me know your feedback?

> Changes since v6:
>  - Addressed review comments from Tejun
> 	- Make locking helpers compact and remove unlock_parent from node.
>  
>  - Addressed review comments from Al Viro
> 	- Make multi node lock helpers non-inline
> 	- Account for change of parent while waiting on semaphores during
> 	  rename operation
> 	- Add a document to describe hashed locks introduced in this patch
> 	  and to provide proof of correctness 
>  
>  - Fix for some issues that I observed while preparing lock correctness
>    document
> 	- Lock both parent and target while looking for symlink
> 	- Introduce a per-fs mutex to synchronize lookup and removal of a node
> 	- Avoid locking parent in remove_self, because only the first instance
> 	  does actual removal so other invocations of remove_self don't need
> 	  to lock the parent
>  
>  - Remove refcounting from lock helpers
> 	- Refcounting was present in lock helpers for cases where an execution
> 	  path acquires node's rwsem and then deletes the node. For such cases
> 	  release helpers should not try to acquire semaphore for this already
> 	  freed node. Refcounting was ensuring that release helpers could get
> 	  an still existing node.
> 	  I have modified locking helpers such that helpers that acquire rwsem
> 	  returns its address which can later be used by release helpers.
> 
[...]
> Imran Khan (8):
>   kernfs: Introduce interface to access global kernfs_open_file_mutex.
>   kernfs: Replace global kernfs_open_file_mutex with hashed mutexes.
>   kernfs: Introduce interface to access kernfs_open_node_lock.
>   kernfs: Replace global kernfs_open_node_lock with hashed spinlocks.
>   kernfs: Use a per-fs rwsem to protect per-fs list of
>     kernfs_super_info.
>   kernfs: Introduce interface to access per-fs rwsem.
>   kernfs: Replace per-fs rwsem with hashed rwsems.
>   kernfs: Add a document to describe hashed locks used in kernfs.
> 
>  .../filesystems/kernfs-hashed-locks.rst       | 245 +++++++++++++++++
>  fs/kernfs/Makefile                            |   2 +-
>  fs/kernfs/dir.c                               | 212 +++++++++-----
>  fs/kernfs/file.c                              |  66 +++--
>  fs/kernfs/inode.c                             |  48 +++-
>  fs/kernfs/kernfs-internal.c                   | 259 ++++++++++++++++++
>  fs/kernfs/kernfs-internal.h                   | 159 +++++++++++
>  fs/kernfs/mount.c                             |  30 +-
>  fs/kernfs/symlink.c                           |  13 +-
>  include/linux/kernfs.h                        |  71 ++++-
>  10 files changed, 980 insertions(+), 125 deletions(-)
>  create mode 100644 Documentation/filesystems/kernfs-hashed-locks.rst
>  create mode 100644 fs/kernfs/kernfs-internal.c
> 
> 
> base-commit: 196d330d7fb1e7cc0d85641c89ce4602cb36f12e

Thanks,
-- Imran