Re: [PATCH 3/9] btrfs: move the tree mod log code into its own file
On 11/3/21 10:31 pm, fdman...@kernel.org wrote: From: Filipe Manana The tree modification log, which records modifications done to btrees, is quite large and currently spread all over ctree.c, which is a huge file already. To make things better organized, move all that code into its own separate source and header files. Functions and definitions that are used outside of the module (mostly by ctree.c) are renamed so that they start with a "btrfs_" prefix. Everything else remains unchanged. This makes it easier to go over the tree modification log code everytime I need to go read it to fix a bug. Signed-off-by: Filipe Manana Reviewed-by: Anand Jain
Re: [PATCH 3/9] btrfs: move the tree mod log code into its own file
On Thu, Mar 11, 2021 at 5:27 PM kernel test robot wrote:
>
> Hi,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on v5.12-rc2]
> [also build test WARNING on next-20210311]
> [cannot apply to kdave/for-next]
> [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-bug-fixes-for-the-tree-mod-log-and-small-refactorings/20210311-223429
> base:a38fd8748464831584a19438cbb3082b5a2dab15
> config: x86_64-randconfig-m001-20210311 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot
>
> smatch warnings:
> fs/btrfs/tree-mod-log.c:286 btrfs_tree_mod_log_insert_move() warn: missing
> error code 'ret'
> fs/btrfs/tree-mod-log.c:519 btrfs_tree_mod_log_eb_copy() warn: missing error
> code 'ret'
> fs/btrfs/tree-mod-log.c:577 btrfs_tree_mod_log_free_eb() warn: missing error
> code 'ret'
No, those are all ok.
For all cases, 'ret' was initialized to 0 when declared, and that's
what we want to return when tree_mod_dont_log() returns true.
>
> vim +/ret +286 fs/btrfs/tree-mod-log.c
>
>246
>247 int btrfs_tree_mod_log_insert_move(struct extent_buffer *eb,
>248 int dst_slot, int src_slot,
>249 int nr_items)
>250 {
>251 struct tree_mod_elem *tm = NULL;
>252 struct tree_mod_elem **tm_list = NULL;
>253 int ret = 0;
>254 int i;
>255 int locked = 0;
>256
>257 if (!tree_mod_need_log(eb->fs_info, eb))
>258 return 0;
>259
>260 tm_list = kcalloc(nr_items, sizeof(struct tree_mod_elem *),
> GFP_NOFS);
>261 if (!tm_list)
>262 return -ENOMEM;
>263
>264 tm = kzalloc(sizeof(*tm), GFP_NOFS);
>265 if (!tm) {
>266 ret = -ENOMEM;
>267 goto free_tms;
>268 }
>269
>270 tm->logical = eb->start;
>271 tm->slot = src_slot;
>272 tm->move.dst_slot = dst_slot;
>273 tm->move.nr_items = nr_items;
>274 tm->op = BTRFS_MOD_LOG_MOVE_KEYS;
>275
>276 for (i = 0; i + dst_slot < src_slot && i < nr_items; i++) {
>277 tm_list[i] = alloc_tree_mod_elem(eb, i + dst_slot,
>278 BTRFS_MOD_LOG_KEY_REMOVE_WHILE_MOVING, GFP_NOFS);
>279 if (!tm_list[i]) {
>280 ret = -ENOMEM;
>281 goto free_tms;
>282 }
>283 }
>284
>285 if (tree_mod_dont_log(eb->fs_info, eb))
> > 286 goto free_tms;
>287 locked = 1;
>288
>289 /*
>290 * When we override something during the move, we log these
> removals.
>291 * This can only happen when we move towards the beginning of
> the
>292 * buffer, i.e. dst_slot < src_slot.
>293 */
>294 for (i = 0; i + dst_slot < src_slot && i < nr_items; i++) {
>295 ret = tree_mod_log_insert(eb->fs_info, tm_list[i]);
>296 if (ret)
>297 goto free_tms;
>298 }
>299
>300 ret = tree_mod_log_insert(eb->fs_info, tm);
>301 if (ret)
>302 goto free_tms;
>303 write_unlock(&eb->fs_info->tree_mod_log_lock);
>304 kfree(tm_list);
>305
>306 return 0;
>307 free_tms:
>308 for (i = 0; i < nr_items; i++) {
>309 if (tm_list[i] && !RB_EMPTY_NODE(&tm_list[i]->node))
>310 rb_erase(&tm_list[i]->node,
> &eb->fs_info->tree_mod_log);
>311 kfree(tm_list[i]);
>312 }
>313 if (locked)
>314 write_unlock(&eb->fs_info->tree_mod_log_lock);
>315 kfree(tm_list);
>316 kfree(tm);
>317
>318 return ret;
>319 }
>320
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH 3/9] btrfs: move the tree mod log code into its own file
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v5.12-rc2]
[also build test WARNING on next-20210311]
[cannot apply to kdave/for-next]
[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-bug-fixes-for-the-tree-mod-log-and-small-refactorings/20210311-223429
base:a38fd8748464831584a19438cbb3082b5a2dab15
config: x86_64-randconfig-m001-20210311 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
smatch warnings:
fs/btrfs/tree-mod-log.c:286 btrfs_tree_mod_log_insert_move() warn: missing
error code 'ret'
fs/btrfs/tree-mod-log.c:519 btrfs_tree_mod_log_eb_copy() warn: missing error
code 'ret'
fs/btrfs/tree-mod-log.c:577 btrfs_tree_mod_log_free_eb() warn: missing error
code 'ret'
vim +/ret +286 fs/btrfs/tree-mod-log.c
246
247 int btrfs_tree_mod_log_insert_move(struct extent_buffer *eb,
248 int dst_slot, int src_slot,
249 int nr_items)
250 {
251 struct tree_mod_elem *tm = NULL;
252 struct tree_mod_elem **tm_list = NULL;
253 int ret = 0;
254 int i;
255 int locked = 0;
256
257 if (!tree_mod_need_log(eb->fs_info, eb))
258 return 0;
259
260 tm_list = kcalloc(nr_items, sizeof(struct tree_mod_elem *),
GFP_NOFS);
261 if (!tm_list)
262 return -ENOMEM;
263
264 tm = kzalloc(sizeof(*tm), GFP_NOFS);
265 if (!tm) {
266 ret = -ENOMEM;
267 goto free_tms;
268 }
269
270 tm->logical = eb->start;
271 tm->slot = src_slot;
272 tm->move.dst_slot = dst_slot;
273 tm->move.nr_items = nr_items;
274 tm->op = BTRFS_MOD_LOG_MOVE_KEYS;
275
276 for (i = 0; i + dst_slot < src_slot && i < nr_items; i++) {
277 tm_list[i] = alloc_tree_mod_elem(eb, i + dst_slot,
278 BTRFS_MOD_LOG_KEY_REMOVE_WHILE_MOVING, GFP_NOFS);
279 if (!tm_list[i]) {
280 ret = -ENOMEM;
281 goto free_tms;
282 }
283 }
284
285 if (tree_mod_dont_log(eb->fs_info, eb))
> 286 goto free_tms;
287 locked = 1;
288
289 /*
290 * When we override something during the move, we log these
removals.
291 * This can only happen when we move towards the beginning of
the
292 * buffer, i.e. dst_slot < src_slot.
293 */
294 for (i = 0; i + dst_slot < src_slot && i < nr_items; i++) {
295 ret = tree_mod_log_insert(eb->fs_info, tm_list[i]);
296 if (ret)
297 goto free_tms;
298 }
299
300 ret = tree_mod_log_insert(eb->fs_info, tm);
301 if (ret)
302 goto free_tms;
303 write_unlock(&eb->fs_info->tree_mod_log_lock);
304 kfree(tm_list);
305
306 return 0;
307 free_tms:
308 for (i = 0; i < nr_items; i++) {
309 if (tm_list[i] && !RB_EMPTY_NODE(&tm_list[i]->node))
310 rb_erase(&tm_list[i]->node,
&eb->fs_info->tree_mod_log);
311 kfree(tm_list[i]);
312 }
313 if (locked)
314 write_unlock(&eb->fs_info->tree_mod_log_lock);
315 kfree(tm_list);
316 kfree(tm);
317
318 return ret;
319 }
320
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
