Skip to contentSkip to conversation

Documentation: filesystems: update filesystem locking documentation

PATCH Superseded Sean Anderson 4 participants 5 messages Jul 31, 2017 – Aug 1, 2017

Subsystems (2)
Documentation VFS
lore
Share & tools
lore

Sean Anderson

author
#0 lore raw

PATCH 1/1 Documentation: filesystems: update filesystem locking documentation

Documentation/filesystems/Locking no longer reflects current locking semantics. i_mutex is no longer used for locking, and has been superseded by i_rwsem. Additionally, ->iterate_shared() was not documented.

Signed-off-by Sean Anderson <[email protected]>
---
 Documentation/filesystems/Locking | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
Documentation/filesystems/Locking +10 -5
index fe25787ff6d4..b098d2e50b7f 100644
@@ -69,8 +69,8 @@ prototypes:
 
 locking rules:
 	all may block
-		i_mutex(inode)
-lookup:		yes
+		i_rwsem(inode)
+lookup:		shared
 create:		yes
 link:		yes (both)
 mknod:		yes

@@ -92,7 +92,7 @@ atomic_open:	yes
 tmpfile:	no
 
 
-	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_mutex on
+	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_rwsem on
 victim.
 	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
 

@@ -111,7 +111,7 @@ prototypes:
 
 locking rules:
 	all may block
-		i_mutex(inode)
+		i_rwsem(inode)
 list:		no
 get:		no
 set:		yes

@@ -217,7 +217,7 @@ prototypes:
 locking rules:
 	All except set_page_dirty and freepage may block
 
-			PageLocked(page)	i_mutex
+			PageLocked(page)	i_rwsem
 writepage:		yes, unlocks (see below)
 readpage:		yes, unlocks
 writepages:

@@ -439,6 +439,7 @@ prototypes:
 	ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
 	ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
 	int (*iterate) (struct file *, struct dir_context *);
+	int (*iterate_shared) (struct file *, struct dir_context *);
 	unsigned int (*poll) (struct file *, struct poll_table_struct *);
 	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
 	long (*compat_ioctl) (struct file *, unsigned int, unsigned long);

@@ -480,6 +481,10 @@ mutex or just to use i_size_read() instead.
 Note: this does not protect the file->f_pos against concurrent modifications
 since this is something the userspace has to take care about.
 
+->iterate() is called with i_rwsem held.
+
+->iterate_shared() is called with i_rwsem shared.
+
 ->fasync() is responsible for maintaining the FASYNC bit in filp->f_flags.
 Most instances call fasync_helper(), which does that maintenance, so it's
 not normally something one needs to worry about.  Return values > 0 will be
Signature · 1 line
2.13.2
#1 lore raw

Documentation/filesystems/Locking prototypes: · line 74

Quoted 5 lines · Sean Anderson
Documentation/filesystems/Locking +2 -2
-		i_mutex(inode)
-lookup:		yes
+		i_rwsem(inode)
+lookup:		shared
 create:		yes

Could you change all the 'yes' to 'exclusive' when it's changed from mutex to rwsem?

Quoted 51 lines

link: yes (both)
mknod: yes

Documentation/filesystems/Locking +8 -3
@@ -92,7 +92,7 @@ atomic_open:	yes
 tmpfile:	no
 
 
-	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_mutex on
+	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_rwsem on
 victim.
 	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
 

@@ -111,7 +111,7 @@ prototypes:
 
 locking rules:
 	all may block
-		i_mutex(inode)
+		i_rwsem(inode)
 list:		no
 get:		no
 set:		yes

@@ -217,7 +217,7 @@ prototypes:
 locking rules:
 	All except set_page_dirty and freepage may block
 
-			PageLocked(page)	i_mutex
+			PageLocked(page)	i_rwsem
 writepage:		yes, unlocks (see below)
 readpage:		yes, unlocks
 writepages:

@@ -439,6 +439,7 @@ prototypes:
 	ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
 	ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
 	int (*iterate) (struct file *, struct dir_context *);
+	int (*iterate_shared) (struct file *, struct dir_context *);
 	unsigned int (*poll) (struct file *, struct poll_table_struct *);
 	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
 	long (*compat_ioctl) (struct file *, unsigned int, unsigned long);

@@ -480,6 +481,10 @@ mutex or just to use i_size_read() instead.
 Note: this does not protect the file->f_pos against concurrent modifications
 since this is something the userspace has to take care about.
 
+->iterate() is called with i_rwsem held.
+
+->iterate_shared() is called with i_rwsem shared.
+
 ->fasync() is responsible for maintaining the FASYNC bit in filp->f_flags.
 Most instances call fasync_helper(), which does that maintenance, so it's
 not normally something one needs to worry about.  Return values > 0 will be
Signature · 1 line
2.13.2
author
#2 lore raw

PATCH v2 1/1 Documentation: filesystems: update filesystem locking documentation

Documentation/filesystems/Locking no longer reflects current locking semantics. i_mutex is no longer used for locking, and has been superseded by i_rwsem. Additionally, ->iterate_shared() was not documented.

Signed-off-by Sean Anderson <[email protected]>

---
v2: changed 'yes's to 'exclusive's when describing i_rwsem usage

 Documentation/filesystems/Locking | 43 ++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 19 deletions(-)
Documentation/filesystems/Locking +24 -19
index fe25787ff6d4..c0cab97d2b1a 100644
@@ -69,31 +69,31 @@ prototypes:
 
 locking rules:
 	all may block
-		i_mutex(inode)
-lookup:		yes
-create:		yes
-link:		yes (both)
-mknod:		yes
-symlink:	yes
-mkdir:		yes
-unlink:		yes (both)
-rmdir:		yes (both)	(see below)
-rename:	yes (all)	(see below)
+		i_rwsem(inode)
+lookup:		shared
+create:		exclusive
+link:		exclusive (both)
+mknod:		exclusive
+symlink:	exclusive
+mkdir:		exclusive
+unlink:		exclusive (both)
+rmdir:		exclusive (both)(see below)
+rename:		exclusive (all)	(see below)
 readlink:	no
 get_link:	no
-setattr:	yes
+setattr:	exclusive
 permission:	no (may not block if called in rcu-walk mode)
 get_acl:	no
 getattr:	no
 listxattr:	no
 fiemap:		no
 update_time:	no
-atomic_open:	yes
+atomic_open:	exclusive
 tmpfile:	no
 
 
-	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_mutex on
-victim.
+	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_rwsem
+	exclusive on victim.
 	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
 
 See Documentation/filesystems/directory-locking for more detailed discussion

@@ -111,10 +111,10 @@ prototypes:
 
 locking rules:
 	all may block
-		i_mutex(inode)
+		i_rwsem(inode)
 list:		no
 get:		no
-set:		yes
+set:		exclusive
 
 --------------------------- super_operations ---------------------------
 prototypes:

@@ -217,14 +217,14 @@ prototypes:
 locking rules:
 	All except set_page_dirty and freepage may block
 
-			PageLocked(page)	i_mutex
+			PageLocked(page)	i_rwsem
 writepage:		yes, unlocks (see below)
 readpage:		yes, unlocks
 writepages:
 set_page_dirty		no
 readpages:
-write_begin:		locks the page		yes
-write_end:		yes, unlocks		yes
+write_begin:		locks the page		exclusive
+write_end:		yes, unlocks		exclusive
 bmap:
 invalidatepage:		yes
 releasepage:		yes

@@ -439,6 +439,7 @@ prototypes:
 	ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
 	ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
 	int (*iterate) (struct file *, struct dir_context *);
+	int (*iterate_shared) (struct file *, struct dir_context *);
 	unsigned int (*poll) (struct file *, struct poll_table_struct *);
 	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
 	long (*compat_ioctl) (struct file *, unsigned int, unsigned long);

@@ -480,6 +481,10 @@ mutex or just to use i_size_read() instead.
 Note: this does not protect the file->f_pos against concurrent modifications
 since this is something the userspace has to take care about.
 
+->iterate() is called with i_rwsem exclusive.
+
+->iterate_shared() is called with i_rwsem at least shared.
+
 ->fasync() is responsible for maintaining the FASYNC bit in filp->f_flags.
 Most instances call fasync_helper(), which does that maintenance, so it's
 not normally something one needs to worry about.  Return values > 0 will be
Signature · 1 line
2.13.2
Reviewed-by on v2 1/1
#3 lore raw
Quoted 101 lines · Sean Anderson

Documentation/filesystems/Locking no longer reflects current locking semantics. i_mutex is no longer used for locking, and has been superseded by i_rwsem. Additionally, ->iterate_shared() was not documented.

Signed-off-by Sean Anderson <[email protected]>

---
v2: changed 'yes's to 'exclusive's when describing i_rwsem usage

 Documentation/filesystems/Locking | 43 ++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 19 deletions(-)
Documentation/filesystems/Locking +20 -19
index fe25787ff6d4..c0cab97d2b1a 100644
@@ -69,31 +69,31 @@ prototypes:
 
 locking rules:
 	all may block
-		i_mutex(inode)
-lookup:		yes
-create:		yes
-link:		yes (both)
-mknod:		yes
-symlink:	yes
-mkdir:		yes
-unlink:		yes (both)
-rmdir:		yes (both)	(see below)
-rename:	yes (all)	(see below)
+		i_rwsem(inode)
+lookup:		shared
+create:		exclusive
+link:		exclusive (both)
+mknod:		exclusive
+symlink:	exclusive
+mkdir:		exclusive
+unlink:		exclusive (both)
+rmdir:		exclusive (both)(see below)
+rename:		exclusive (all)	(see below)
 readlink:	no
 get_link:	no
-setattr:	yes
+setattr:	exclusive
 permission:	no (may not block if called in rcu-walk mode)
 get_acl:	no
 getattr:	no
 listxattr:	no
 fiemap:		no
 update_time:	no
-atomic_open:	yes
+atomic_open:	exclusive
 tmpfile:	no
 
 
-	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_mutex on
-victim.
+	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_rwsem
+	exclusive on victim.
 	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
 
 See Documentation/filesystems/directory-locking for more detailed discussion

@@ -111,10 +111,10 @@ prototypes:
 
 locking rules:
 	all may block
-		i_mutex(inode)
+		i_rwsem(inode)
 list:		no
 get:		no
-set:		yes
+set:		exclusive
 
 --------------------------- super_operations ---------------------------
 prototypes:

@@ -217,14 +217,14 @@ prototypes:
 locking rules:
 	All except set_page_dirty and freepage may block
 
-			PageLocked(page)	i_mutex
+			PageLocked(page)	i_rwsem
 writepage:		yes, unlocks (see below)
 readpage:		yes, unlocks
 writepages:
 set_page_dirty		no
 readpages:
-write_begin:		locks the page		yes
-write_end:		yes, unlocks		yes
+write_begin:		locks the page		exclusive
+write_end:		yes, unlocks		exclusive
 bmap:
 invalidatepage:		yes
 releasepage:		yes

@@ -439,6 +439,7 @@ prototypes:
 	ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
 	ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
 	int (*iterate) (struct file *, struct dir_context *);
+	int (*iterate_shared) (struct file *, struct dir_context *);
 	unsigned int (*poll) (struct file *, struct poll_table_struct *);
 	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
 	long (*compat_ioctl) (struct file *, unsigned int, unsigned long);

Documentation/filesystems/Locking mutex or just to use i_size_read() instead. · line 490

Quoted 10 lines
Documentation/filesystems/Locking +4 -0
@@ -480,6 +481,10 @@ mutex or just to use i_size_read() instead.
 Note: this does not protect the file->f_pos against concurrent modifications
 since this is something the userspace has to take care about.
 
+->iterate() is called with i_rwsem exclusive.
+
+->iterate_shared() is called with i_rwsem at least shared.
+
 ->fasync() is responsible for maintaining the FASYNC bit in filp->f_flags.
 Most instances call fasync_helper(), which does that maintenance, so it's
 not normally something one needs to worry about.  Return values > 0 will be
Reviewed-by Jeff Layton <[email protected]>
maintainer
#4 lore raw
In reply to Sean Anderson
Quoted 5 lines · On Tue, 1 Aug 2017 07:09:10 -0400 Sean Anderson <[email protected]> wrote:

Documentation/filesystems/Locking no longer reflects current locking semantics. i_mutex is no longer used for locking, and has been superseded by i_rwsem. Additionally, ->iterate_shared() was not documented.

Signed-off-by Sean Anderson <[email protected]>

So should I pick this up, or are we waiting for Al to finish unpacking ... ?

jon