[Btrfs-devel] Some queries about map_extent_buffer()

[Peter Teoh]

In many parts of code map_extent_buffer() return value is not handled.
  What is this function doing?   Essentially calling kmap_atomic()
using the KM_USER1 memory, right?   This memory is only one page in
size, and so it may failed easily?

                        map_extent_buffer(parent,
                                        btrfs_node_key_ptr_offset(i),
                                        sizeof(struct btrfs_key_ptr),
                                        &parent->map_token, &parent->kaddr,
                                        &parent->map_start, &parent->map_len,
                                        KM_USER1);

Personally, I don't think this API is wise to use kmap_atomic(), as in
many parts of kernel codes, in between kmap_ and kunmap_ it is just a
few instructions from each other.   But here I can see like this:

ctree.c:

                if (!parent->map_token) {
                        map_extent_buffer(parent,
                                        btrfs_node_key_ptr_offset(i),
                                        sizeof(struct btrfs_key_ptr),
                                        &parent->map_token, &parent->kaddr,
                                        &parent->map_start, &parent->map_len,
                                        KM_USER1);
                }
                btrfs_node_key(parent, &disk_key, i);
                if (!progress_passed && comp_keys(&disk_key, progress) < 0)
                        continue;

                progress_passed = 1;
                blocknr = btrfs_node_blockptr(parent, i);
                if (last_block == 0)
                        last_block = blocknr;

                if (i > 0) {
                        other = btrfs_node_blockptr(parent, i - 1);
                        close = close_blocks(blocknr, other, blocksize);
                }
                if (close && i < end_slot - 2) {
                        other = btrfs_node_blockptr(parent, i + 1);
                        close = close_blocks(blocknr, other, blocksize);
                }
                if (close) {
                        last_block = blocknr;
                        continue;
                }
                if (parent->map_token) {
                        unmap_extent_buffer(parent, parent->map_token,
                                            KM_USER1);
                        parent->map_token = NULL;
                }

                cur = btrfs_find_tree_block(root, blocknr, blocksize);

So problems like leakage can easily occurred.   For example, after the
map_extent_buffer(), followed by the "continue", the
unmap_extent_buffer() will not be called, a problem???? I am not sure,
as I don't fully understand the code yet :-).   (And the
map_extent_buffer will be called again, using KM_USER1, without the
previous one being unmap?).



-- 
Regards,
Peter Teoh