From: kernel test robot <lkp@intel.com> To: kbuild@lists.01.org Subject: Re: [PATCH 3/3] btrfs: do not pin logs too early during renames Date: Thu, 29 Jul 2021 06:04:28 +0800 [thread overview] Message-ID: <202107290512.NuBwLok7-lkp@intel.com> (raw) [-- Attachment #1: Type: text/plain, Size: 21494 bytes --] CC: kbuild-all(a)lists.01.org In-Reply-To: <019dba0981817da70e0820550267b368e3ce7389.1627379796.git.fdmanana@suse.com> References: <019dba0981817da70e0820550267b368e3ce7389.1627379796.git.fdmanana@suse.com> TO: fdmanana(a)kernel.org Hi, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on kdave/for-next] [also build test WARNING on v5.14-rc3 next-20210727] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/fdmanana-kernel-org/btrfs-fsync-changes-a-bug-fix-and-a-couple-improvements/20210727-182628 base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next :::::: branch date: 2 days ago :::::: commit date: 2 days ago config: arm64-randconfig-m031-20210728 (attached as .config) compiler: aarch64-linux-gcc (GCC) 10.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> New smatch warnings: fs/btrfs/inode.c:9338 btrfs_rename_exchange() warn: variable dereferenced before check 'new_inode' (see line 9225) Old smatch warnings: fs/btrfs/inode.c:285 insert_inline_extent() error: we previously assumed 'compressed_pages' could be null (see line 254) vim +/new_inode +9338 fs/btrfs/inode.c 39279cc3d2704c Chris Mason 2007-06-12 9117 cdd1fedf8261cd Dan Fuhry 2016-03-17 9118 static int btrfs_rename_exchange(struct inode *old_dir, cdd1fedf8261cd Dan Fuhry 2016-03-17 9119 struct dentry *old_dentry, cdd1fedf8261cd Dan Fuhry 2016-03-17 9120 struct inode *new_dir, cdd1fedf8261cd Dan Fuhry 2016-03-17 9121 struct dentry *new_dentry) cdd1fedf8261cd Dan Fuhry 2016-03-17 9122 { 0b246afa62b0cf Jeff Mahoney 2016-06-22 9123 struct btrfs_fs_info *fs_info = btrfs_sb(old_dir->i_sb); cdd1fedf8261cd Dan Fuhry 2016-03-17 9124 struct btrfs_trans_handle *trans; cdd1fedf8261cd Dan Fuhry 2016-03-17 9125 struct btrfs_root *root = BTRFS_I(old_dir)->root; cdd1fedf8261cd Dan Fuhry 2016-03-17 9126 struct btrfs_root *dest = BTRFS_I(new_dir)->root; cdd1fedf8261cd Dan Fuhry 2016-03-17 9127 struct inode *new_inode = new_dentry->d_inode; cdd1fedf8261cd Dan Fuhry 2016-03-17 9128 struct inode *old_inode = old_dentry->d_inode; 95582b00838837 Deepa Dinamani 2018-05-08 9129 struct timespec64 ctime = current_time(old_inode); 4a0cc7ca6c40b6 Nikolay Borisov 2017-01-10 9130 u64 old_ino = btrfs_ino(BTRFS_I(old_inode)); 4a0cc7ca6c40b6 Nikolay Borisov 2017-01-10 9131 u64 new_ino = btrfs_ino(BTRFS_I(new_inode)); cdd1fedf8261cd Dan Fuhry 2016-03-17 9132 u64 old_idx = 0; cdd1fedf8261cd Dan Fuhry 2016-03-17 9133 u64 new_idx = 0; cdd1fedf8261cd Dan Fuhry 2016-03-17 9134 int ret; 75b463d2b47aef Filipe Manana 2020-08-11 9135 int ret2; 86e8aa0e772cab Filipe Manana 2016-05-05 9136 bool root_log_pinned = false; 86e8aa0e772cab Filipe Manana 2016-05-05 9137 bool dest_log_pinned = false; dc09ef3562726c Josef Bacik 2021-05-19 9138 bool need_abort = false; cdd1fedf8261cd Dan Fuhry 2016-03-17 9139 cdd1fedf8261cd Dan Fuhry 2016-03-17 9140 /* we only allow rename subvolume link between subvolumes */ cdd1fedf8261cd Dan Fuhry 2016-03-17 9141 if (old_ino != BTRFS_FIRST_FREE_OBJECTID && root != dest) cdd1fedf8261cd Dan Fuhry 2016-03-17 9142 return -EXDEV; cdd1fedf8261cd Dan Fuhry 2016-03-17 9143 cdd1fedf8261cd Dan Fuhry 2016-03-17 9144 /* close the race window with snapshot create/destroy ioctl */ 943eb3bf25f4a7 Josef Bacik 2019-11-19 9145 if (old_ino == BTRFS_FIRST_FREE_OBJECTID || 943eb3bf25f4a7 Josef Bacik 2019-11-19 9146 new_ino == BTRFS_FIRST_FREE_OBJECTID) 0b246afa62b0cf Jeff Mahoney 2016-06-22 9147 down_read(&fs_info->subvol_sem); cdd1fedf8261cd Dan Fuhry 2016-03-17 9148 cdd1fedf8261cd Dan Fuhry 2016-03-17 9149 /* cdd1fedf8261cd Dan Fuhry 2016-03-17 9150 * We want to reserve the absolute worst case amount of items. So if cdd1fedf8261cd Dan Fuhry 2016-03-17 9151 * both inodes are subvols and we need to unlink them then that would cdd1fedf8261cd Dan Fuhry 2016-03-17 9152 * require 4 item modifications, but if they are both normal inodes it cdd1fedf8261cd Dan Fuhry 2016-03-17 9153 * would require 5 item modifications, so we'll assume their normal cdd1fedf8261cd Dan Fuhry 2016-03-17 9154 * inodes. So 5 * 2 is 10, plus 2 for the new links, so 12 total items cdd1fedf8261cd Dan Fuhry 2016-03-17 9155 * should cover the worst case number of items we'll modify. cdd1fedf8261cd Dan Fuhry 2016-03-17 9156 */ cdd1fedf8261cd Dan Fuhry 2016-03-17 9157 trans = btrfs_start_transaction(root, 12); cdd1fedf8261cd Dan Fuhry 2016-03-17 9158 if (IS_ERR(trans)) { cdd1fedf8261cd Dan Fuhry 2016-03-17 9159 ret = PTR_ERR(trans); cdd1fedf8261cd Dan Fuhry 2016-03-17 9160 goto out_notrans; cdd1fedf8261cd Dan Fuhry 2016-03-17 9161 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9162 00aa8e87c9dc63 Josef Bacik 2021-03-12 9163 if (dest != root) { 00aa8e87c9dc63 Josef Bacik 2021-03-12 9164 ret = btrfs_record_root_in_trans(trans, dest); 00aa8e87c9dc63 Josef Bacik 2021-03-12 9165 if (ret) 00aa8e87c9dc63 Josef Bacik 2021-03-12 9166 goto out_fail; 00aa8e87c9dc63 Josef Bacik 2021-03-12 9167 } 3e1740993e4311 Josef Bacik 2019-11-15 9168 cdd1fedf8261cd Dan Fuhry 2016-03-17 9169 /* cdd1fedf8261cd Dan Fuhry 2016-03-17 9170 * We need to find a free sequence number both in the source and cdd1fedf8261cd Dan Fuhry 2016-03-17 9171 * in the destination directory for the exchange. cdd1fedf8261cd Dan Fuhry 2016-03-17 9172 */ 877574e2548bbf Nikolay Borisov 2017-02-20 9173 ret = btrfs_set_inode_index(BTRFS_I(new_dir), &old_idx); cdd1fedf8261cd Dan Fuhry 2016-03-17 9174 if (ret) cdd1fedf8261cd Dan Fuhry 2016-03-17 9175 goto out_fail; 877574e2548bbf Nikolay Borisov 2017-02-20 9176 ret = btrfs_set_inode_index(BTRFS_I(old_dir), &new_idx); cdd1fedf8261cd Dan Fuhry 2016-03-17 9177 if (ret) cdd1fedf8261cd Dan Fuhry 2016-03-17 9178 goto out_fail; cdd1fedf8261cd Dan Fuhry 2016-03-17 9179 cdd1fedf8261cd Dan Fuhry 2016-03-17 9180 BTRFS_I(old_inode)->dir_index = 0ULL; cdd1fedf8261cd Dan Fuhry 2016-03-17 9181 BTRFS_I(new_inode)->dir_index = 0ULL; cdd1fedf8261cd Dan Fuhry 2016-03-17 9182 cdd1fedf8261cd Dan Fuhry 2016-03-17 9183 /* Reference for the source. */ cdd1fedf8261cd Dan Fuhry 2016-03-17 9184 if (old_ino == BTRFS_FIRST_FREE_OBJECTID) { cdd1fedf8261cd Dan Fuhry 2016-03-17 9185 /* force full log commit if subvolume involved. */ 907877664e2d85 David Sterba 2019-03-20 9186 btrfs_set_log_full_commit(trans); cdd1fedf8261cd Dan Fuhry 2016-03-17 9187 } else { cdd1fedf8261cd Dan Fuhry 2016-03-17 9188 ret = btrfs_insert_inode_ref(trans, dest, cdd1fedf8261cd Dan Fuhry 2016-03-17 9189 new_dentry->d_name.name, cdd1fedf8261cd Dan Fuhry 2016-03-17 9190 new_dentry->d_name.len, cdd1fedf8261cd Dan Fuhry 2016-03-17 9191 old_ino, f85b7379cd76ad David Sterba 2017-01-20 9192 btrfs_ino(BTRFS_I(new_dir)), f85b7379cd76ad David Sterba 2017-01-20 9193 old_idx); cdd1fedf8261cd Dan Fuhry 2016-03-17 9194 if (ret) cdd1fedf8261cd Dan Fuhry 2016-03-17 9195 goto out_fail; dc09ef3562726c Josef Bacik 2021-05-19 9196 need_abort = true; cdd1fedf8261cd Dan Fuhry 2016-03-17 9197 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9198 cdd1fedf8261cd Dan Fuhry 2016-03-17 9199 /* And now for the dest. */ cdd1fedf8261cd Dan Fuhry 2016-03-17 9200 if (new_ino == BTRFS_FIRST_FREE_OBJECTID) { cdd1fedf8261cd Dan Fuhry 2016-03-17 9201 /* force full log commit if subvolume involved. */ 907877664e2d85 David Sterba 2019-03-20 9202 btrfs_set_log_full_commit(trans); cdd1fedf8261cd Dan Fuhry 2016-03-17 9203 } else { cdd1fedf8261cd Dan Fuhry 2016-03-17 9204 ret = btrfs_insert_inode_ref(trans, root, cdd1fedf8261cd Dan Fuhry 2016-03-17 9205 old_dentry->d_name.name, cdd1fedf8261cd Dan Fuhry 2016-03-17 9206 old_dentry->d_name.len, cdd1fedf8261cd Dan Fuhry 2016-03-17 9207 new_ino, f85b7379cd76ad David Sterba 2017-01-20 9208 btrfs_ino(BTRFS_I(old_dir)), f85b7379cd76ad David Sterba 2017-01-20 9209 new_idx); dc09ef3562726c Josef Bacik 2021-05-19 9210 if (ret) { dc09ef3562726c Josef Bacik 2021-05-19 9211 if (need_abort) dc09ef3562726c Josef Bacik 2021-05-19 9212 btrfs_abort_transaction(trans, ret); cdd1fedf8261cd Dan Fuhry 2016-03-17 9213 goto out_fail; cdd1fedf8261cd Dan Fuhry 2016-03-17 9214 } dc09ef3562726c Josef Bacik 2021-05-19 9215 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9216 cdd1fedf8261cd Dan Fuhry 2016-03-17 9217 /* Update inode version and ctime/mtime. */ cdd1fedf8261cd Dan Fuhry 2016-03-17 9218 inode_inc_iversion(old_dir); cdd1fedf8261cd Dan Fuhry 2016-03-17 9219 inode_inc_iversion(new_dir); cdd1fedf8261cd Dan Fuhry 2016-03-17 9220 inode_inc_iversion(old_inode); cdd1fedf8261cd Dan Fuhry 2016-03-17 9221 inode_inc_iversion(new_inode); cdd1fedf8261cd Dan Fuhry 2016-03-17 9222 old_dir->i_ctime = old_dir->i_mtime = ctime; cdd1fedf8261cd Dan Fuhry 2016-03-17 9223 new_dir->i_ctime = new_dir->i_mtime = ctime; cdd1fedf8261cd Dan Fuhry 2016-03-17 9224 old_inode->i_ctime = ctime; cdd1fedf8261cd Dan Fuhry 2016-03-17 @9225 new_inode->i_ctime = ctime; cdd1fedf8261cd Dan Fuhry 2016-03-17 9226 cdd1fedf8261cd Dan Fuhry 2016-03-17 9227 if (old_dentry->d_parent != new_dentry->d_parent) { f85b7379cd76ad David Sterba 2017-01-20 9228 btrfs_record_unlink_dir(trans, BTRFS_I(old_dir), f85b7379cd76ad David Sterba 2017-01-20 9229 BTRFS_I(old_inode), 1); f85b7379cd76ad David Sterba 2017-01-20 9230 btrfs_record_unlink_dir(trans, BTRFS_I(new_dir), f85b7379cd76ad David Sterba 2017-01-20 9231 BTRFS_I(new_inode), 1); cdd1fedf8261cd Dan Fuhry 2016-03-17 9232 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9233 c8758bb7bea761 Filipe Manana 2021-07-27 9234 /* c8758bb7bea761 Filipe Manana 2021-07-27 9235 * Now pin the logs of the roots. We do it to ensure that no other task c8758bb7bea761 Filipe Manana 2021-07-27 9236 * can sync the logs while we are in progress with the rename, because c8758bb7bea761 Filipe Manana 2021-07-27 9237 * that could result in an inconsistency in case any of the inodes that c8758bb7bea761 Filipe Manana 2021-07-27 9238 * are part of this rename operation were logged before. c8758bb7bea761 Filipe Manana 2021-07-27 9239 * c8758bb7bea761 Filipe Manana 2021-07-27 9240 * We pin the logs even if at this precise moment none of the inodes was c8758bb7bea761 Filipe Manana 2021-07-27 9241 * logged before. This is because right after we checked for that, some c8758bb7bea761 Filipe Manana 2021-07-27 9242 * other task fsyncing some other inode not involved with this rename c8758bb7bea761 Filipe Manana 2021-07-27 9243 * operation could log that one of our inodes exists. c8758bb7bea761 Filipe Manana 2021-07-27 9244 * c8758bb7bea761 Filipe Manana 2021-07-27 9245 * We don't need to pin the logs before the above calls to c8758bb7bea761 Filipe Manana 2021-07-27 9246 * btrfs_insert_inode_ref(), since those don't ever need to change a log. c8758bb7bea761 Filipe Manana 2021-07-27 9247 */ c8758bb7bea761 Filipe Manana 2021-07-27 9248 if (old_ino != BTRFS_FIRST_FREE_OBJECTID) { c8758bb7bea761 Filipe Manana 2021-07-27 9249 btrfs_pin_log_trans(root); c8758bb7bea761 Filipe Manana 2021-07-27 9250 root_log_pinned = true; c8758bb7bea761 Filipe Manana 2021-07-27 9251 } c8758bb7bea761 Filipe Manana 2021-07-27 9252 if (new_ino != BTRFS_FIRST_FREE_OBJECTID) { c8758bb7bea761 Filipe Manana 2021-07-27 9253 btrfs_pin_log_trans(dest); c8758bb7bea761 Filipe Manana 2021-07-27 9254 dest_log_pinned = true; c8758bb7bea761 Filipe Manana 2021-07-27 9255 } c8758bb7bea761 Filipe Manana 2021-07-27 9256 cdd1fedf8261cd Dan Fuhry 2016-03-17 9257 /* src is a subvolume */ cdd1fedf8261cd Dan Fuhry 2016-03-17 9258 if (old_ino == BTRFS_FIRST_FREE_OBJECTID) { 045d3967b6920b Josef Bacik 2019-12-18 9259 ret = btrfs_unlink_subvol(trans, old_dir, old_dentry); cdd1fedf8261cd Dan Fuhry 2016-03-17 9260 } else { /* src is an inode */ 4ec5934e43cabd Nikolay Borisov 2017-01-18 9261 ret = __btrfs_unlink_inode(trans, root, BTRFS_I(old_dir), 4ec5934e43cabd Nikolay Borisov 2017-01-18 9262 BTRFS_I(old_dentry->d_inode), cdd1fedf8261cd Dan Fuhry 2016-03-17 9263 old_dentry->d_name.name, cdd1fedf8261cd Dan Fuhry 2016-03-17 9264 old_dentry->d_name.len); cdd1fedf8261cd Dan Fuhry 2016-03-17 9265 if (!ret) 9a56fcd15a9c6b Nikolay Borisov 2020-11-02 9266 ret = btrfs_update_inode(trans, root, BTRFS_I(old_inode)); cdd1fedf8261cd Dan Fuhry 2016-03-17 9267 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9268 if (ret) { 66642832f06a43 Jeff Mahoney 2016-06-10 9269 btrfs_abort_transaction(trans, ret); cdd1fedf8261cd Dan Fuhry 2016-03-17 9270 goto out_fail; cdd1fedf8261cd Dan Fuhry 2016-03-17 9271 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9272 cdd1fedf8261cd Dan Fuhry 2016-03-17 9273 /* dest is a subvolume */ cdd1fedf8261cd Dan Fuhry 2016-03-17 9274 if (new_ino == BTRFS_FIRST_FREE_OBJECTID) { 045d3967b6920b Josef Bacik 2019-12-18 9275 ret = btrfs_unlink_subvol(trans, new_dir, new_dentry); cdd1fedf8261cd Dan Fuhry 2016-03-17 9276 } else { /* dest is an inode */ 4ec5934e43cabd Nikolay Borisov 2017-01-18 9277 ret = __btrfs_unlink_inode(trans, dest, BTRFS_I(new_dir), 4ec5934e43cabd Nikolay Borisov 2017-01-18 9278 BTRFS_I(new_dentry->d_inode), cdd1fedf8261cd Dan Fuhry 2016-03-17 9279 new_dentry->d_name.name, cdd1fedf8261cd Dan Fuhry 2016-03-17 9280 new_dentry->d_name.len); cdd1fedf8261cd Dan Fuhry 2016-03-17 9281 if (!ret) 9a56fcd15a9c6b Nikolay Borisov 2020-11-02 9282 ret = btrfs_update_inode(trans, dest, BTRFS_I(new_inode)); cdd1fedf8261cd Dan Fuhry 2016-03-17 9283 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9284 if (ret) { 66642832f06a43 Jeff Mahoney 2016-06-10 9285 btrfs_abort_transaction(trans, ret); cdd1fedf8261cd Dan Fuhry 2016-03-17 9286 goto out_fail; cdd1fedf8261cd Dan Fuhry 2016-03-17 9287 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9288 db0a669fb00241 Nikolay Borisov 2017-02-20 9289 ret = btrfs_add_link(trans, BTRFS_I(new_dir), BTRFS_I(old_inode), cdd1fedf8261cd Dan Fuhry 2016-03-17 9290 new_dentry->d_name.name, cdd1fedf8261cd Dan Fuhry 2016-03-17 9291 new_dentry->d_name.len, 0, old_idx); cdd1fedf8261cd Dan Fuhry 2016-03-17 9292 if (ret) { 66642832f06a43 Jeff Mahoney 2016-06-10 9293 btrfs_abort_transaction(trans, ret); cdd1fedf8261cd Dan Fuhry 2016-03-17 9294 goto out_fail; cdd1fedf8261cd Dan Fuhry 2016-03-17 9295 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9296 db0a669fb00241 Nikolay Borisov 2017-02-20 9297 ret = btrfs_add_link(trans, BTRFS_I(old_dir), BTRFS_I(new_inode), cdd1fedf8261cd Dan Fuhry 2016-03-17 9298 old_dentry->d_name.name, cdd1fedf8261cd Dan Fuhry 2016-03-17 9299 old_dentry->d_name.len, 0, new_idx); cdd1fedf8261cd Dan Fuhry 2016-03-17 9300 if (ret) { 66642832f06a43 Jeff Mahoney 2016-06-10 9301 btrfs_abort_transaction(trans, ret); cdd1fedf8261cd Dan Fuhry 2016-03-17 9302 goto out_fail; cdd1fedf8261cd Dan Fuhry 2016-03-17 9303 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9304 cdd1fedf8261cd Dan Fuhry 2016-03-17 9305 if (old_inode->i_nlink == 1) cdd1fedf8261cd Dan Fuhry 2016-03-17 9306 BTRFS_I(old_inode)->dir_index = old_idx; cdd1fedf8261cd Dan Fuhry 2016-03-17 9307 if (new_inode->i_nlink == 1) cdd1fedf8261cd Dan Fuhry 2016-03-17 9308 BTRFS_I(new_inode)->dir_index = new_idx; cdd1fedf8261cd Dan Fuhry 2016-03-17 9309 86e8aa0e772cab Filipe Manana 2016-05-05 9310 if (root_log_pinned) { 75b463d2b47aef Filipe Manana 2020-08-11 9311 btrfs_log_new_name(trans, BTRFS_I(old_inode), BTRFS_I(old_dir), 75b463d2b47aef Filipe Manana 2020-08-11 9312 new_dentry->d_parent); cdd1fedf8261cd Dan Fuhry 2016-03-17 9313 btrfs_end_log_trans(root); 86e8aa0e772cab Filipe Manana 2016-05-05 9314 root_log_pinned = false; cdd1fedf8261cd Dan Fuhry 2016-03-17 9315 } 86e8aa0e772cab Filipe Manana 2016-05-05 9316 if (dest_log_pinned) { 75b463d2b47aef Filipe Manana 2020-08-11 9317 btrfs_log_new_name(trans, BTRFS_I(new_inode), BTRFS_I(new_dir), 75b463d2b47aef Filipe Manana 2020-08-11 9318 old_dentry->d_parent); cdd1fedf8261cd Dan Fuhry 2016-03-17 9319 btrfs_end_log_trans(dest); 86e8aa0e772cab Filipe Manana 2016-05-05 9320 dest_log_pinned = false; cdd1fedf8261cd Dan Fuhry 2016-03-17 9321 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9322 out_fail: 86e8aa0e772cab Filipe Manana 2016-05-05 9323 /* 86e8aa0e772cab Filipe Manana 2016-05-05 9324 * If we have pinned a log and an error happened, we unpin tasks 86e8aa0e772cab Filipe Manana 2016-05-05 9325 * trying to sync the log and force them to fallback to a transaction 86e8aa0e772cab Filipe Manana 2016-05-05 9326 * commit if the log currently contains any of the inodes involved in 86e8aa0e772cab Filipe Manana 2016-05-05 9327 * this rename operation (to ensure we do not persist a log with an 86e8aa0e772cab Filipe Manana 2016-05-05 9328 * inconsistent state for any of these inodes or leading to any 86e8aa0e772cab Filipe Manana 2016-05-05 9329 * inconsistencies when replayed). If the transaction was aborted, the 86e8aa0e772cab Filipe Manana 2016-05-05 9330 * abortion reason is propagated to userspace when attempting to commit 86e8aa0e772cab Filipe Manana 2016-05-05 9331 * the transaction. If the log does not contain any of these inodes, we 86e8aa0e772cab Filipe Manana 2016-05-05 9332 * allow the tasks to sync it. 86e8aa0e772cab Filipe Manana 2016-05-05 9333 */ 86e8aa0e772cab Filipe Manana 2016-05-05 9334 if (ret && (root_log_pinned || dest_log_pinned)) { 0f8939b8ac8623 Nikolay Borisov 2017-01-18 9335 if (btrfs_inode_in_log(BTRFS_I(old_dir), fs_info->generation) || 0f8939b8ac8623 Nikolay Borisov 2017-01-18 9336 btrfs_inode_in_log(BTRFS_I(new_dir), fs_info->generation) || 0f8939b8ac8623 Nikolay Borisov 2017-01-18 9337 btrfs_inode_in_log(BTRFS_I(old_inode), fs_info->generation) || 86e8aa0e772cab Filipe Manana 2016-05-05 @9338 (new_inode && 0f8939b8ac8623 Nikolay Borisov 2017-01-18 9339 btrfs_inode_in_log(BTRFS_I(new_inode), fs_info->generation))) 907877664e2d85 David Sterba 2019-03-20 9340 btrfs_set_log_full_commit(trans); 86e8aa0e772cab Filipe Manana 2016-05-05 9341 86e8aa0e772cab Filipe Manana 2016-05-05 9342 if (root_log_pinned) { 86e8aa0e772cab Filipe Manana 2016-05-05 9343 btrfs_end_log_trans(root); 86e8aa0e772cab Filipe Manana 2016-05-05 9344 root_log_pinned = false; 86e8aa0e772cab Filipe Manana 2016-05-05 9345 } 86e8aa0e772cab Filipe Manana 2016-05-05 9346 if (dest_log_pinned) { 86e8aa0e772cab Filipe Manana 2016-05-05 9347 btrfs_end_log_trans(dest); 86e8aa0e772cab Filipe Manana 2016-05-05 9348 dest_log_pinned = false; 86e8aa0e772cab Filipe Manana 2016-05-05 9349 } 86e8aa0e772cab Filipe Manana 2016-05-05 9350 } c5b4a50b74018b Filipe Manana 2018-06-11 9351 ret2 = btrfs_end_transaction(trans); c5b4a50b74018b Filipe Manana 2018-06-11 9352 ret = ret ? ret : ret2; cdd1fedf8261cd Dan Fuhry 2016-03-17 9353 out_notrans: 943eb3bf25f4a7 Josef Bacik 2019-11-19 9354 if (new_ino == BTRFS_FIRST_FREE_OBJECTID || 943eb3bf25f4a7 Josef Bacik 2019-11-19 9355 old_ino == BTRFS_FIRST_FREE_OBJECTID) 0b246afa62b0cf Jeff Mahoney 2016-06-22 9356 up_read(&fs_info->subvol_sem); cdd1fedf8261cd Dan Fuhry 2016-03-17 9357 cdd1fedf8261cd Dan Fuhry 2016-03-17 9358 return ret; cdd1fedf8261cd Dan Fuhry 2016-03-17 9359 } cdd1fedf8261cd Dan Fuhry 2016-03-17 9360 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 37662 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com> To: kbuild-all@lists.01.org Subject: Re: [PATCH 3/3] btrfs: do not pin logs too early during renames Date: Thu, 29 Jul 2021 12:27:13 +0300 [thread overview] Message-ID: <202107290512.NuBwLok7-lkp@intel.com> (raw) In-Reply-To: <019dba0981817da70e0820550267b368e3ce7389.1627379796.git.fdmanana@suse.com> [-- Attachment #1: Type: text/plain, Size: 6208 bytes --] [ I don't know why Smatch is suddenly complaining about this code when it's 5 years old. But we may as well remove the NULL check. - dan] Hi, url: https://github.com/0day-ci/linux/commits/fdmanana-kernel-org/btrfs-fsync-changes-a-bug-fix-and-a-couple-improvements/20210727-182628 base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next config: arm64-randconfig-m031-20210728 (attached as .config) compiler: aarch64-linux-gcc (GCC) 10.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> New smatch warnings: fs/btrfs/inode.c:9338 btrfs_rename_exchange() warn: variable dereferenced before check 'new_inode' (see line 9225) Old smatch warnings: fs/btrfs/inode.c:285 insert_inline_extent() error: we previously assumed 'compressed_pages' could be null (see line 254) vim +/new_inode +9338 fs/btrfs/inode.c cdd1fedf8261cd Dan Fuhry 2016-03-17 9118 static int btrfs_rename_exchange(struct inode *old_dir, cdd1fedf8261cd Dan Fuhry 2016-03-17 9119 struct dentry *old_dentry, cdd1fedf8261cd Dan Fuhry 2016-03-17 9120 struct inode *new_dir, cdd1fedf8261cd Dan Fuhry 2016-03-17 9121 struct dentry *new_dentry) cdd1fedf8261cd Dan Fuhry 2016-03-17 9122 { 0b246afa62b0cf Jeff Mahoney 2016-06-22 9123 struct btrfs_fs_info *fs_info = btrfs_sb(old_dir->i_sb); cdd1fedf8261cd Dan Fuhry 2016-03-17 9124 struct btrfs_trans_handle *trans; cdd1fedf8261cd Dan Fuhry 2016-03-17 9125 struct btrfs_root *root = BTRFS_I(old_dir)->root; cdd1fedf8261cd Dan Fuhry 2016-03-17 9126 struct btrfs_root *dest = BTRFS_I(new_dir)->root; cdd1fedf8261cd Dan Fuhry 2016-03-17 9127 struct inode *new_inode = new_dentry->d_inode; cdd1fedf8261cd Dan Fuhry 2016-03-17 9128 struct inode *old_inode = old_dentry->d_inode; 95582b00838837 Deepa Dinamani 2018-05-08 9129 struct timespec64 ctime = current_time(old_inode); 4a0cc7ca6c40b6 Nikolay Borisov 2017-01-10 9130 u64 old_ino = btrfs_ino(BTRFS_I(old_inode)); 4a0cc7ca6c40b6 Nikolay Borisov 2017-01-10 9131 u64 new_ino = btrfs_ino(BTRFS_I(new_inode)); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "new_inode" can't be NULL. Or more properly we are toasted if it is. cdd1fedf8261cd Dan Fuhry 2016-03-17 9132 u64 old_idx = 0; cdd1fedf8261cd Dan Fuhry 2016-03-17 9133 u64 new_idx = 0; cdd1fedf8261cd Dan Fuhry 2016-03-17 9134 int ret; [ snip ] cdd1fedf8261cd Dan Fuhry 2016-03-17 9322 out_fail: 86e8aa0e772cab Filipe Manana 2016-05-05 9323 /* 86e8aa0e772cab Filipe Manana 2016-05-05 9324 * If we have pinned a log and an error happened, we unpin tasks 86e8aa0e772cab Filipe Manana 2016-05-05 9325 * trying to sync the log and force them to fallback to a transaction 86e8aa0e772cab Filipe Manana 2016-05-05 9326 * commit if the log currently contains any of the inodes involved in 86e8aa0e772cab Filipe Manana 2016-05-05 9327 * this rename operation (to ensure we do not persist a log with an 86e8aa0e772cab Filipe Manana 2016-05-05 9328 * inconsistent state for any of these inodes or leading to any 86e8aa0e772cab Filipe Manana 2016-05-05 9329 * inconsistencies when replayed). If the transaction was aborted, the 86e8aa0e772cab Filipe Manana 2016-05-05 9330 * abortion reason is propagated to userspace when attempting to commit 86e8aa0e772cab Filipe Manana 2016-05-05 9331 * the transaction. If the log does not contain any of these inodes, we 86e8aa0e772cab Filipe Manana 2016-05-05 9332 * allow the tasks to sync it. 86e8aa0e772cab Filipe Manana 2016-05-05 9333 */ 86e8aa0e772cab Filipe Manana 2016-05-05 9334 if (ret && (root_log_pinned || dest_log_pinned)) { 0f8939b8ac8623 Nikolay Borisov 2017-01-18 9335 if (btrfs_inode_in_log(BTRFS_I(old_dir), fs_info->generation) || 0f8939b8ac8623 Nikolay Borisov 2017-01-18 9336 btrfs_inode_in_log(BTRFS_I(new_dir), fs_info->generation) || 0f8939b8ac8623 Nikolay Borisov 2017-01-18 9337 btrfs_inode_in_log(BTRFS_I(old_inode), fs_info->generation) || 86e8aa0e772cab Filipe Manana 2016-05-05 @9338 (new_inode && ^^^^^^^^^ This check is not required and only upsets the static checker. 0f8939b8ac8623 Nikolay Borisov 2017-01-18 9339 btrfs_inode_in_log(BTRFS_I(new_inode), fs_info->generation))) 907877664e2d85 David Sterba 2019-03-20 9340 btrfs_set_log_full_commit(trans); 86e8aa0e772cab Filipe Manana 2016-05-05 9341 86e8aa0e772cab Filipe Manana 2016-05-05 9342 if (root_log_pinned) { 86e8aa0e772cab Filipe Manana 2016-05-05 9343 btrfs_end_log_trans(root); 86e8aa0e772cab Filipe Manana 2016-05-05 9344 root_log_pinned = false; 86e8aa0e772cab Filipe Manana 2016-05-05 9345 } 86e8aa0e772cab Filipe Manana 2016-05-05 9346 if (dest_log_pinned) { 86e8aa0e772cab Filipe Manana 2016-05-05 9347 btrfs_end_log_trans(dest); 86e8aa0e772cab Filipe Manana 2016-05-05 9348 dest_log_pinned = false; 86e8aa0e772cab Filipe Manana 2016-05-05 9349 } 86e8aa0e772cab Filipe Manana 2016-05-05 9350 } c5b4a50b74018b Filipe Manana 2018-06-11 9351 ret2 = btrfs_end_transaction(trans); c5b4a50b74018b Filipe Manana 2018-06-11 9352 ret = ret ? ret : ret2; cdd1fedf8261cd Dan Fuhry 2016-03-17 9353 out_notrans: 943eb3bf25f4a7 Josef Bacik 2019-11-19 9354 if (new_ino == BTRFS_FIRST_FREE_OBJECTID || 943eb3bf25f4a7 Josef Bacik 2019-11-19 9355 old_ino == BTRFS_FIRST_FREE_OBJECTID) 0b246afa62b0cf Jeff Mahoney 2016-06-22 9356 up_read(&fs_info->subvol_sem); cdd1fedf8261cd Dan Fuhry 2016-03-17 9357 cdd1fedf8261cd Dan Fuhry 2016-03-17 9358 return ret; cdd1fedf8261cd Dan Fuhry 2016-03-17 9359 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
next reply other threads:[~2021-07-28 22:04 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-28 22:04 kernel test robot [this message] 2021-07-29 9:27 ` [PATCH 3/3] btrfs: do not pin logs too early during renames Dan Carpenter 2021-07-29 14:40 ` Filipe Manana -- strict thread matches above, loose matches on Subject: below -- 2021-07-27 10:24 [PATCH 0/3] btrfs: fsync changes, a bug fix and a couple improvements fdmanana 2021-07-27 10:24 ` [PATCH 1/3] btrfs: fix lost inode on log replay after mix of fsync, rename and inode eviction fdmanana 2021-07-27 10:24 ` [PATCH 2/3] btrfs: eliminate some false positives when checking if inode was logged fdmanana 2021-07-27 10:24 ` [PATCH 3/3] btrfs: do not pin logs too early during renames fdmanana 2021-07-28 12:41 ` [PATCH 0/3] btrfs: fsync changes, a bug fix and a couple improvements David Sterba
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=202107290512.NuBwLok7-lkp@intel.com \ --to=lkp@intel.com \ --cc=kbuild@lists.01.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.