On 2019/2/21 下午10:13, Dan Carpenter wrote:
> [ This one is not actually your fault. It's the core code which is
> confusing. -dan ]
>
> Hello Qu Wenruo,
>
> The patch b72c3aba09a5: "btrfs: locking: Add extra check in
> btrfs_init_new_buffer() to avoid deadlock" from Aug 21, 2018, leads
> to the following static checker warning:
>
> fs/btrfs/extent-tree.c:8556 btrfs_init_new_buffer()
> warn: possible NULL dereference of 'buf'
>
> fs/btrfs/extent-tree.c
> 8540 static struct extent_buffer *
> 8541 btrfs_init_new_buffer(struct btrfs_trans_handle *trans, struct
> btrfs_root *root,
> 8542 u64 bytenr, int level, u64 owner)
> 8543 {
> 8544 struct btrfs_fs_info *fs_info = root->fs_info;
> 8545 struct extent_buffer *buf;
> 8546
> 8547 buf = btrfs_find_create_tree_block(fs_info, bytenr);
>
> The btrfs_find_create_tree_block() function either returns
> alloc_test_extent_buffer() which returns NULL on error
You caught me!
Indeed that's a possible NULL return case.
The only good news is it shouldn't affect release build which normally
doesn't enabled CONFIG_BTRFS_FS_RUN_SANITY_TESTS.
I'll fix it by unifying the return pointer.
Thanks for pointing this out,
Qu
> or
> alloc_extent_buffer() which returns error pointers on error. It
> confuses static checkers and me also.
>
> 8548 if (IS_ERR(buf))
> 8549 return buf;
> 8550
> 8551 /*
> 8552 * Extra safety check in case the extent tree is corrupted and
> extent
> 8553 * allocator chooses to use a tree block which is already used
> and
> 8554 * locked.
> 8555 */
> --> 8556 if (buf->lock_owner == current->pid) {
> 8557 btrfs_err_rl(fs_info,
> 8558 "tree block %llu owner %llu already locked by pid=%d, extent tree
> corruption detected",
> 8559 buf->start, btrfs_header_owner(buf),
> current->pid);
> 8560 free_extent_buffer(buf);
>
> regards,
> dan carpenter
>
signature.asc
Description: OpenPGP digital signature
