diff mbox series

mm: Fully initialize invalidate_lock, amend lock class later

Message ID 20210901084403.g4fezi23cixemlhh@linutronix.de (mailing list archive)
State New, archived
Headers show
Series mm: Fully initialize invalidate_lock, amend lock class later | expand

Commit Message

Sebastian Andrzej Siewior Sept. 1, 2021, 8:44 a.m. UTC
The function __init_rwsem() is not part of the official API, it just a helper
function used by init_rwsem().
Changing the lock's class and name should be done by using
lockdep_set_class_and_name() after the has been fully initialized. The overhead
of the additional class struct and setting it twice is negligible and it works
across all locks.

Fully initialize the lock with init_rwsem() and then set the custom class and
name for the lock.

Fixes: 730633f0b7f95 ("mm: Protect operations adding pages to page cache with invalidate_lock")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 fs/inode.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Sebastian Andrzej Siewior Sept. 17, 2021, 10:34 a.m. UTC | #1
On 2021-09-01 10:44:04 [+0200], To linux-fsdevel@vger.kernel.org wrote:
> The function __init_rwsem() is not part of the official API, it just a helper
> function used by init_rwsem().
> Changing the lock's class and name should be done by using
> lockdep_set_class_and_name() after the has been fully initialized. The overhead
> of the additional class struct and setting it twice is negligible and it works
> across all locks.
> 
> Fully initialize the lock with init_rwsem() and then set the custom class and
> name for the lock.
> 
> Fixes: 730633f0b7f95 ("mm: Protect operations adding pages to page cache with invalidate_lock")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

ping.

> ---
>  fs/inode.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index cb41f02d8cedf..a49695f57e1ea 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -190,8 +190,10 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
>  	mapping_set_gfp_mask(mapping, GFP_HIGHUSER_MOVABLE);
>  	mapping->private_data = NULL;
>  	mapping->writeback_index = 0;
> -	__init_rwsem(&mapping->invalidate_lock, "mapping.invalidate_lock",
> -		     &sb->s_type->invalidate_lock_key);
> +	init_rwsem(&mapping->invalidate_lock);
> +	lockdep_set_class_and_name(&mapping->invalidate_lock,
> +				   &sb->s_type->invalidate_lock_key,
> +				   "mapping.invalidate_lock");
>  	inode->i_private = NULL;
>  	inode->i_mapping = mapping;
>  	INIT_HLIST_HEAD(&inode->i_dentry);	/* buggered by rcu freeing */

Sebastian
Jan Kara Sept. 17, 2021, 11:39 a.m. UTC | #2
On Wed 01-09-21 10:44:03, Sebastian Andrzej Siewior wrote:
> The function __init_rwsem() is not part of the official API, it just a helper
> function used by init_rwsem().
> Changing the lock's class and name should be done by using
> lockdep_set_class_and_name() after the has been fully initialized. The overhead
> of the additional class struct and setting it twice is negligible and it works
> across all locks.
> 
> Fully initialize the lock with init_rwsem() and then set the custom class and
> name for the lock.
> 
> Fixes: 730633f0b7f95 ("mm: Protect operations adding pages to page cache with invalidate_lock")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Fine by me, thanks for the fix. I'll queue it to my tree and push it to
Linus.

								Honza

> ---
>  fs/inode.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index cb41f02d8cedf..a49695f57e1ea 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -190,8 +190,10 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
>  	mapping_set_gfp_mask(mapping, GFP_HIGHUSER_MOVABLE);
>  	mapping->private_data = NULL;
>  	mapping->writeback_index = 0;
> -	__init_rwsem(&mapping->invalidate_lock, "mapping.invalidate_lock",
> -		     &sb->s_type->invalidate_lock_key);
> +	init_rwsem(&mapping->invalidate_lock);
> +	lockdep_set_class_and_name(&mapping->invalidate_lock,
> +				   &sb->s_type->invalidate_lock_key,
> +				   "mapping.invalidate_lock");
>  	inode->i_private = NULL;
>  	inode->i_mapping = mapping;
>  	INIT_HLIST_HEAD(&inode->i_dentry);	/* buggered by rcu freeing */
> -- 
> 2.33.0
>
diff mbox series

Patch

diff --git a/fs/inode.c b/fs/inode.c
index cb41f02d8cedf..a49695f57e1ea 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -190,8 +190,10 @@  int inode_init_always(struct super_block *sb, struct inode *inode)
 	mapping_set_gfp_mask(mapping, GFP_HIGHUSER_MOVABLE);
 	mapping->private_data = NULL;
 	mapping->writeback_index = 0;
-	__init_rwsem(&mapping->invalidate_lock, "mapping.invalidate_lock",
-		     &sb->s_type->invalidate_lock_key);
+	init_rwsem(&mapping->invalidate_lock);
+	lockdep_set_class_and_name(&mapping->invalidate_lock,
+				   &sb->s_type->invalidate_lock_key,
+				   "mapping.invalidate_lock");
 	inode->i_private = NULL;
 	inode->i_mapping = mapping;
 	INIT_HLIST_HEAD(&inode->i_dentry);	/* buggered by rcu freeing */