Hi, Dan Carpenter Thanks for reporting this problem. Keeping code in good logic can reduce bug and make it easy to maintenance. > -----Original Message----- > From: linux-btrfs-owner@xxxxxxxxxxxxxxx > [mailto:linux-btrfs-owner@xxxxxxxxxxxxxxx] On Behalf Of Dan Carpenter > Sent: Friday, August 14, 2015 5:28 PM > To: miaox@xxxxxxxxxxxxxx > Cc: linux-btrfs@xxxxxxxxxxxxxxx > Subject: re: Btrfs: don't start the log transaction if the log tree init fails > > Hello Miao Xie, > > The patch e87ac1368700: "Btrfs: don't start the log transaction if the log tree > init fails" from Feb 20, 2014, leads to the following static checker warning: > > fs/btrfs/tree-log.c:178 start_log_trans() > warn: we tested 'root->log_root' before and it was 'false' > > fs/btrfs/tree-log.c > 147 if (root->log_root) { > > We test "root->log_root" here. > > 148 if (btrfs_need_log_full_commit(root->fs_info, > trans)) { > 149 ret = -EAGAIN; > 150 goto out; > 151 } > 152 if (!root->log_start_pid) { > 153 root->log_start_pid = current->pid; > 154 > clear_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state); > 155 } else if (root->log_start_pid != current->pid) { > 156 > set_bit(BTRFS_ROOT_MULTI_LOG_TASKS, &root->state); > 157 } > 158 > 159 atomic_inc(&root->log_batch); > 160 atomic_inc(&root->log_writers); > 161 if (ctx) { > 162 index = root->log_transid % 2; > 163 list_add_tail(&ctx->list, > &root->log_ctxs[index]); > 164 ctx->log_transid = root->log_transid; > 165 } > 166 mutex_unlock(&root->log_mutex); > 167 return 0; > 168 } > 169 > 170 ret = 0; > 171 mutex_lock(&root->fs_info->tree_log_mutex); > 172 if (!root->fs_info->log_root_tree) > 173 ret = btrfs_init_log_root_tree(trans, > root->fs_info); > 174 mutex_unlock(&root->fs_info->tree_log_mutex); > 175 if (ret) > 176 goto out; > 177 > 178 if (!root->log_root) { > > Couldn't we just remove this condition here? This is a new Smatch thing I am > working on and I am investigating false positives. > Above problem was introduced by commit: 7237f1833, and this patch(e87ac1368700) make the problem easy to find out. We can remove above condition to make code pass smatch checking, but a better way is to do some cleanup to remove duplicated code in above function. I'll fix it. Thanks Zhaolei > 179 ret = btrfs_add_log_tree(trans, root); > 180 if (ret) > 181 goto out; > 182 } > > regards, > dan carpenter > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body > of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at > http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
