diff mbox series

[v2,3/4] fs: add check functions for sb_start_{write,pagefault,intwrite}

Message ID 0737603ecc6baf785843d6e91992e6ef202c308c.1647436353.git.naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series protect relocation with sb_start_write | expand

Commit Message

Naohiro Aota March 16, 2022, 1:22 p.m. UTC 
Add a function sb_write_started() to return if sb_start_write() is
properly called. It is used in the next commit.

Also, add the similar functions for sb_start_pagefault() and
sb_start_intwrite().

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 include/linux/fs.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Dave Chinner March 17, 2022, 8:34 a.m. UTC | #1 
On Wed, Mar 16, 2022 at 10:22:39PM +0900, Naohiro Aota wrote:
> Add a function sb_write_started() to return if sb_start_write() is
> properly called. It is used in the next commit.
> 
> Also, add the similar functions for sb_start_pagefault() and
> sb_start_intwrite().
> 
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  include/linux/fs.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 27746a3da8fd..0c8714d64169 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1732,6 +1732,11 @@ static inline bool __sb_start_write_trylock(struct super_block *sb, int level)
>  #define __sb_writers_release(sb, lev)	\
>  	percpu_rwsem_release(&(sb)->s_writers.rw_sem[(lev)-1], 1, _THIS_IP_)
>  
> +static inline bool __sb_write_started(struct super_block *sb, int level)
> +{
> +	return lockdep_is_held_type(sb->s_writers.rw_sem + level - 1, 1);
> +}
> +
>  /**
>   * sb_end_write - drop write access to a superblock
>   * @sb: the super we wrote to
> @@ -1797,6 +1802,11 @@ static inline bool sb_start_write_trylock(struct super_block *sb)
>  	return __sb_start_write_trylock(sb, SB_FREEZE_WRITE);
>  }
>  
> +static inline bool sb_write_started(struct super_block *sb)
> +{
> +	return __sb_write_started(sb, SB_FREEZE_WRITE);
> +}
> +
>  /**
>   * sb_start_pagefault - get write access to a superblock from a page fault
>   * @sb: the super we write to
> @@ -1821,6 +1831,11 @@ static inline void sb_start_pagefault(struct super_block *sb)
>  	__sb_start_write(sb, SB_FREEZE_PAGEFAULT);
>  }
>  
> +static inline bool sb_pagefault_started(struct super_block *sb)
> +{
> +	return __sb_write_started(sb, SB_FREEZE_PAGEFAULT);
> +}
> +
>  /**
>   * sb_start_intwrite - get write access to a superblock for internal fs purposes
>   * @sb: the super we write to
> @@ -1844,6 +1859,11 @@ static inline bool sb_start_intwrite_trylock(struct super_block *sb)
>  	return __sb_start_write_trylock(sb, SB_FREEZE_FS);
>  }
>  
> +static inline bool sb_intwrite_started(struct super_block *sb)
> +{
> +	return __sb_write_started(sb, SB_FREEZE_FS);
> +}
> +
>  bool inode_owner_or_capable(struct user_namespace *mnt_userns,
>  			    const struct inode *inode);
>  

We should not be adding completely unused code to the VFS APIs.

Just rename __sb_write_started() to sb_write_started() and pass
SB_FREEZE_WRITE directly from the single debug assert that you've
added that needs this check.

Cheers,

Dave.
Naohiro Aota March 17, 2022, 11:13 a.m. UTC | #2 
On Thu, Mar 17, 2022 at 07:34:20PM +1100, Dave Chinner wrote:
> On Wed, Mar 16, 2022 at 10:22:39PM +0900, Naohiro Aota wrote:
> > Add a function sb_write_started() to return if sb_start_write() is
> > properly called. It is used in the next commit.
> > 
> > Also, add the similar functions for sb_start_pagefault() and
> > sb_start_intwrite().
> > 
> > Reviewed-by: Filipe Manana <fdmanana@suse.com>
> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> > ---
> >  include/linux/fs.h | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 27746a3da8fd..0c8714d64169 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1732,6 +1732,11 @@ static inline bool __sb_start_write_trylock(struct super_block *sb, int level)
> >  #define __sb_writers_release(sb, lev)	\
> >  	percpu_rwsem_release(&(sb)->s_writers.rw_sem[(lev)-1], 1, _THIS_IP_)
> >  
> > +static inline bool __sb_write_started(struct super_block *sb, int level)
> > +{
> > +	return lockdep_is_held_type(sb->s_writers.rw_sem + level - 1, 1);
> > +}
> > +
> >  /**
> >   * sb_end_write - drop write access to a superblock
> >   * @sb: the super we wrote to
> > @@ -1797,6 +1802,11 @@ static inline bool sb_start_write_trylock(struct super_block *sb)
> >  	return __sb_start_write_trylock(sb, SB_FREEZE_WRITE);
> >  }
> >  
> > +static inline bool sb_write_started(struct super_block *sb)
> > +{
> > +	return __sb_write_started(sb, SB_FREEZE_WRITE);
> > +}
> > +
> >  /**
> >   * sb_start_pagefault - get write access to a superblock from a page fault
> >   * @sb: the super we write to
> > @@ -1821,6 +1831,11 @@ static inline void sb_start_pagefault(struct super_block *sb)
> >  	__sb_start_write(sb, SB_FREEZE_PAGEFAULT);
> >  }
> >  
> > +static inline bool sb_pagefault_started(struct super_block *sb)
> > +{
> > +	return __sb_write_started(sb, SB_FREEZE_PAGEFAULT);
> > +}
> > +
> >  /**
> >   * sb_start_intwrite - get write access to a superblock for internal fs purposes
> >   * @sb: the super we write to
> > @@ -1844,6 +1859,11 @@ static inline bool sb_start_intwrite_trylock(struct super_block *sb)
> >  	return __sb_start_write_trylock(sb, SB_FREEZE_FS);
> >  }
> >  
> > +static inline bool sb_intwrite_started(struct super_block *sb)
> > +{
> > +	return __sb_write_started(sb, SB_FREEZE_FS);
> > +}
> > +
> >  bool inode_owner_or_capable(struct user_namespace *mnt_userns,
> >  			    const struct inode *inode);
> >  
> 
> We should not be adding completely unused code to the VFS APIs.
> 
> Just rename __sb_write_started() to sb_write_started() and pass
> SB_FREEZE_WRITE directly from the single debug assert that you've
> added that needs this check.

Sure, I will drop them in the next version. I just added them for
completeness.

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
diff mbox series

Patch

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 27746a3da8fd..0c8714d64169 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1732,6 +1732,11 @@  static inline bool __sb_start_write_trylock(struct super_block *sb, int level)
 #define __sb_writers_release(sb, lev)	\
 	percpu_rwsem_release(&(sb)->s_writers.rw_sem[(lev)-1], 1, _THIS_IP_)
 
+static inline bool __sb_write_started(struct super_block *sb, int level)
+{
+	return lockdep_is_held_type(sb->s_writers.rw_sem + level - 1, 1);
+}
+
 /**
  * sb_end_write - drop write access to a superblock
  * @sb: the super we wrote to
@@ -1797,6 +1802,11 @@  static inline bool sb_start_write_trylock(struct super_block *sb)
 	return __sb_start_write_trylock(sb, SB_FREEZE_WRITE);
 }
 
+static inline bool sb_write_started(struct super_block *sb)
+{
+	return __sb_write_started(sb, SB_FREEZE_WRITE);
+}
+
 /**
  * sb_start_pagefault - get write access to a superblock from a page fault
  * @sb: the super we write to
@@ -1821,6 +1831,11 @@  static inline void sb_start_pagefault(struct super_block *sb)
 	__sb_start_write(sb, SB_FREEZE_PAGEFAULT);
 }
 
+static inline bool sb_pagefault_started(struct super_block *sb)
+{
+	return __sb_write_started(sb, SB_FREEZE_PAGEFAULT);
+}
+
 /**
  * sb_start_intwrite - get write access to a superblock for internal fs purposes
  * @sb: the super we write to
@@ -1844,6 +1859,11 @@  static inline bool sb_start_intwrite_trylock(struct super_block *sb)
 	return __sb_start_write_trylock(sb, SB_FREEZE_FS);
 }
 
+static inline bool sb_intwrite_started(struct super_block *sb)
+{
+	return __sb_write_started(sb, SB_FREEZE_FS);
+}
+
 bool inode_owner_or_capable(struct user_namespace *mnt_userns,
 			    const struct inode *inode);