[bug report] ext4: hold s_fc_lock while during fast commit

[bug report] ext4: hold s_fc_lock while during fast commit

[Dan Carpenter]

Hello Harshad Shirwadkar,

Commit 6593714d67ba ("ext4: hold s_fc_lock while during fast commit")
from May 8, 2025 (linux-next), leads to the following Smatch static
checker warning:

	fs/ext4/fast_commit.c:1159 ext4_fc_perform_commit()
	warn: double unlock '&sbi->s_fc_lock' (orig line 1109)

fs/ext4/fast_commit.c
    1039 static int ext4_fc_perform_commit(journal_t *journal)
    1040 {
    1041         struct super_block *sb = journal->j_private;
    1042         struct ext4_sb_info *sbi = EXT4_SB(sb);
    1043         struct ext4_inode_info *iter;
    1044         struct ext4_fc_head head;
    1045         struct inode *inode;
    1046         struct blk_plug plug;
    1047         int ret = 0;
    1048         u32 crc = 0;
    1049 
    1050         /*
    1051          * Step 1: Mark all inodes on s_fc_q[MAIN] with
    1052          * EXT4_STATE_FC_FLUSHING_DATA. This prevents these inodes from being
    1053          * freed until the data flush is over.
    1054          */
    1055         mutex_lock(&sbi->s_fc_lock);
    1056         list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
    1057                 ext4_set_inode_state(&iter->vfs_inode,
    1058                                      EXT4_STATE_FC_FLUSHING_DATA);
    1059         }
    1060         mutex_unlock(&sbi->s_fc_lock);
    1061 
    1062         /* Step 2: Flush data for all the eligible inodes. */
    1063         ret = ext4_fc_flush_data(journal);
    1064 
    1065         /*
    1066          * Step 3: Clear EXT4_STATE_FC_FLUSHING_DATA flag, before returning
    1067          * any error from step 2. This ensures that waiters waiting on
    1068          * EXT4_STATE_FC_FLUSHING_DATA can resume.
    1069          */
    1070         mutex_lock(&sbi->s_fc_lock);
    1071         list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
    1072                 ext4_clear_inode_state(&iter->vfs_inode,
    1073                                        EXT4_STATE_FC_FLUSHING_DATA);
    1074 #if (BITS_PER_LONG < 64)
    1075                 wake_up_bit(&iter->i_state_flags, EXT4_STATE_FC_FLUSHING_DATA);
    1076 #else
    1077                 wake_up_bit(&iter->i_flags, EXT4_STATE_FC_FLUSHING_DATA);
    1078 #endif
    1079         }
    1080 
    1081         /*
    1082          * Make sure clearing of EXT4_STATE_FC_FLUSHING_DATA is visible before
    1083          * the waiter checks the bit. Pairs with implicit barrier in
    1084          * prepare_to_wait() in ext4_fc_del().
    1085          */
    1086         smp_mb();
    1087         mutex_unlock(&sbi->s_fc_lock);
    1088 
    1089         /*
    1090          * If we encountered error in Step 2, return it now after clearing
    1091          * EXT4_STATE_FC_FLUSHING_DATA bit.
    1092          */
    1093         if (ret)
    1094                 return ret;
    1095 
    1096 
    1097         /* Step 4: Mark all inodes as being committed. */
    1098         jbd2_journal_lock_updates(journal);
    1099         /*
    1100          * The journal is now locked. No more handles can start and all the
    1101          * previous handles are now drained. We now mark the inodes on the
    1102          * commit queue as being committed.
    1103          */
    1104         mutex_lock(&sbi->s_fc_lock);
    1105         list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
    1106                 ext4_set_inode_state(&iter->vfs_inode,
    1107                                      EXT4_STATE_FC_COMMITTING);
    1108         }
    1109         mutex_unlock(&sbi->s_fc_lock);
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    1110         jbd2_journal_unlock_updates(journal);
    1111 
    1112         /*
    1113          * Step 5: If file system device is different from journal device,
    1114          * issue a cache flush before we start writing fast commit blocks.
    1115          */
    1116         if (journal->j_fs_dev != journal->j_dev)
    1117                 blkdev_issue_flush(journal->j_fs_dev);
    1118 
    1119         blk_start_plug(&plug);
    1120         /* Step 6: Write fast commit blocks to disk. */
    1121         if (sbi->s_fc_bytes == 0) {
    1122                 /*
    1123                  * Step 6.1: Add a head tag only if this is the first fast
    1124                  * commit in this TID.
    1125                  */
    1126                 head.fc_features = cpu_to_le32(EXT4_FC_SUPPORTED_FEATURES);
    1127                 head.fc_tid = cpu_to_le32(
    1128                         sbi->s_journal->j_running_transaction->t_tid);
    1129                 if (!ext4_fc_add_tlv(sb, EXT4_FC_TAG_HEAD, sizeof(head),
    1130                         (u8 *)&head, &crc)) {
    1131                         ret = -ENOSPC;
    1132                         goto out;
                                 ^^^^^^^^^
We dropped the lock already.

    1133                 }
    1134         }
    1135 
    1136         /* Step 6.2: Now write all the dentry updates. */
    1137         mutex_lock(&sbi->s_fc_lock);
    1138         ret = ext4_fc_commit_dentry_updates(journal, &crc);
    1139         if (ret)
    1140                 goto out;
    1141 
    1142         /* Step 6.3: Now write all the changed inodes to disk. */
    1143         list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
    1144                 inode = &iter->vfs_inode;
    1145                 if (!ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
    1146                         continue;
    1147 
    1148                 ret = ext4_fc_write_inode_data(inode, &crc);
    1149                 if (ret)
    1150                         goto out;
    1151                 ret = ext4_fc_write_inode(inode, &crc);
    1152                 if (ret)
    1153                         goto out;
    1154         }
    1155         /* Step 6.4: Finally write tail tag to conclude this fast commit. */
    1156         ret = ext4_fc_write_tail(sb, crc);
    1157 
    1158 out:
--> 1159         mutex_unlock(&sbi->s_fc_lock);

Double unlock.

    1160         blk_finish_plug(&plug);
    1161         return ret;
    1162 }

regards,
dan carpenter