[PATCH 1/2] ext4: fix sparse warnings in fast_commit code

[PATCH 1/2] ext4: fix sparse warnings in fast_commit code

[Theodore Ts'o]

Add missing __acquire() and __releases() annotations, and make
fc_ineligible_reasons[] static, as it is not used outside of
fs/ext4/fast_commit.c.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/fast_commit.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 5cd6630ab1b9..f2033e13a273 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -156,6 +156,7 @@ void ext4_fc_init_inode(struct inode *inode)
 
 /* This function must be called with sbi->s_fc_lock held. */
 static void ext4_fc_wait_committing_inode(struct inode *inode)
+__releases(&EXT4_SB(inode->i_sb)->s_fc_lock)
 {
 	wait_queue_head_t *wq;
 	struct ext4_inode_info *ei = EXT4_I(inode);
@@ -911,6 +912,8 @@ static int ext4_fc_wait_inode_data_all(journal_t *journal)
 
 /* Commit all the directory entry updates */
 static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc)
+__acquires(&sbi->s_fc_lock)
+__releases(&sbi->s_fc_lock)
 {
 	struct super_block *sb = (struct super_block *)(journal->j_private);
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
@@ -2106,7 +2109,7 @@ void ext4_fc_init(struct super_block *sb, journal_t *journal)
 	journal->j_fc_cleanup_callback = ext4_fc_cleanup;
 }
 
-const char *fc_ineligible_reasons[] = {
+static const char *fc_ineligible_reasons[] = {
 	"Extended attributes changed",
 	"Cross rename",
 	"Journal flag changed",
-- 
2.28.0

[PATCH 2/2] jbd2: fix up sparse warnings in checkpoint code

[Theodore Ts'o]

Add missing __acquires() and __releases() annotations.  Also, in an
"this should never happen" WARN_ON check, if it *does* actually
happen, we need to release j_state_lock since this function is always
supposed to release that lock.  Otherwise, things will quickly grind
to a halt after the WARN_ON trips.

Fixes: 96f1e0974575 ("jbd2: avoid long hold times of j_state_lock...")
Cc: stable@kernel.org
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/jbd2/checkpoint.c  | 2 ++
 fs/jbd2/transaction.c | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 263f02ad8ebf..472932b9e6bc 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -106,6 +106,8 @@ static int __try_to_free_cp_buf(struct journal_head *jh)
  * for a checkpoint to free up some space in the log.
  */
 void __jbd2_log_wait_for_space(journal_t *journal)
+__acquires(&journal->j_state_lock)
+__releases(&journal->j_state_lock)
 {
 	int nblocks, space_left;
 	/* assert_spin_locked(&journal->j_state_lock); */
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 43985738aa86..d54f04674e8e 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -195,8 +195,10 @@ static void wait_transaction_switching(journal_t *journal)
 	DEFINE_WAIT(wait);
 
 	if (WARN_ON(!journal->j_running_transaction ||
-		    journal->j_running_transaction->t_state != T_SWITCH))
+		    journal->j_running_transaction->t_state != T_SWITCH)) {
+		read_unlock(&journal->j_state_lock);
 		return;
+	}
 	prepare_to_wait(&journal->j_wait_transaction_locked, &wait,
 			TASK_UNINTERRUPTIBLE);
 	read_unlock(&journal->j_state_lock);
-- 
2.28.0

Re: [PATCH 1/2] ext4: fix sparse warnings in fast_commit code

[harshad shirwadkar]

Thanks for the change, it looks good to me.

- Harshad

On Fri, Nov 6, 2020 at 9:14 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> Add missing __acquire() and __releases() annotations, and make
> fc_ineligible_reasons[] static, as it is not used outside of
> fs/ext4/fast_commit.c.
>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  fs/ext4/fast_commit.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 5cd6630ab1b9..f2033e13a273 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -156,6 +156,7 @@ void ext4_fc_init_inode(struct inode *inode)
>
>  /* This function must be called with sbi->s_fc_lock held. */
>  static void ext4_fc_wait_committing_inode(struct inode *inode)
> +__releases(&EXT4_SB(inode->i_sb)->s_fc_lock)
>  {
>         wait_queue_head_t *wq;
>         struct ext4_inode_info *ei = EXT4_I(inode);
> @@ -911,6 +912,8 @@ static int ext4_fc_wait_inode_data_all(journal_t *journal)
>
>  /* Commit all the directory entry updates */
>  static int ext4_fc_commit_dentry_updates(journal_t *journal, u32 *crc)
> +__acquires(&sbi->s_fc_lock)
> +__releases(&sbi->s_fc_lock)
>  {
>         struct super_block *sb = (struct super_block *)(journal->j_private);
>         struct ext4_sb_info *sbi = EXT4_SB(sb);
> @@ -2106,7 +2109,7 @@ void ext4_fc_init(struct super_block *sb, journal_t *journal)
>         journal->j_fc_cleanup_callback = ext4_fc_cleanup;
>  }
>
> -const char *fc_ineligible_reasons[] = {
> +static const char *fc_ineligible_reasons[] = {
>         "Extended attributes changed",
>         "Cross rename",
>         "Journal flag changed",
> --
> 2.28.0
>

Re: [PATCH 2/2] jbd2: fix up sparse warnings in checkpoint code

[harshad shirwadkar]

Thanks for the change, it looks good to me.

- Harshad

On Fri, Nov 6, 2020 at 9:12 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> Add missing __acquires() and __releases() annotations.  Also, in an
> "this should never happen" WARN_ON check, if it *does* actually
> happen, we need to release j_state_lock since this function is always
> supposed to release that lock.  Otherwise, things will quickly grind
> to a halt after the WARN_ON trips.
>
> Fixes: 96f1e0974575 ("jbd2: avoid long hold times of j_state_lock...")
> Cc: stable@kernel.org
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  fs/jbd2/checkpoint.c  | 2 ++
>  fs/jbd2/transaction.c | 4 +++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index 263f02ad8ebf..472932b9e6bc 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -106,6 +106,8 @@ static int __try_to_free_cp_buf(struct journal_head *jh)
>   * for a checkpoint to free up some space in the log.
>   */
>  void __jbd2_log_wait_for_space(journal_t *journal)
> +__acquires(&journal->j_state_lock)
> +__releases(&journal->j_state_lock)
>  {
>         int nblocks, space_left;
>         /* assert_spin_locked(&journal->j_state_lock); */
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 43985738aa86..d54f04674e8e 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -195,8 +195,10 @@ static void wait_transaction_switching(journal_t *journal)
>         DEFINE_WAIT(wait);
>
>         if (WARN_ON(!journal->j_running_transaction ||
> -                   journal->j_running_transaction->t_state != T_SWITCH))
> +                   journal->j_running_transaction->t_state != T_SWITCH)) {
> +               read_unlock(&journal->j_state_lock);
>                 return;
> +       }
>         prepare_to_wait(&journal->j_wait_transaction_locked, &wait,
>                         TASK_UNINTERRUPTIBLE);
>         read_unlock(&journal->j_state_lock);
> --
> 2.28.0
>