btrfs_close_devices() should reset fs_devices->latest_bdev and fs_devices->lowest_bdev as well they hold fd of the open dev in the list which is being closed. On 07/03/2013 09:25 PM, Miao Xie wrote:
Some commands(such as btrfs-convert) access the devices again after we close the ctree, so it is better that we don't free the devices objects when the ctree is closed, or we need re-allocate the memory for the devices.
We needn't worry the memory leak problem, because all the memory will be freed after the taskes die.
That will apply same for close as well right ? However from debugging and converting these functions as library-functions point of view its better if we have close/free called explicitly where possible. I am sending a patch to fix the the memory leak. So its fine if this patch just address the close issue. Thanks, Anand
Signed-off-by: Miao Xie <mi...@cn.fujitsu.com> --- btrfs-find-root.c | 21 +-------------------- disk-io.c | 30 ++---------------------------- volumes.c | 3 +++ 3 files changed, 6 insertions(+), 48 deletions(-) diff --git a/btrfs-find-root.c b/btrfs-find-root.c index 3e1396d..da22c1d 100644 --- a/btrfs-find-root.c +++ b/btrfs-find-root.c @@ -65,25 +65,6 @@ int csum_block(void *buf, u32 len) return ret; } -static int close_all_devices(struct btrfs_fs_info *fs_info) -{ - struct list_head *list; - struct list_head *next; - struct btrfs_device *device; - - return 0; - - list = &fs_info->fs_devices->devices; - list_for_each(next, list) { - device = list_entry(next, struct btrfs_device, dev_list); - if (device->fd != -1) { - close(device->fd); - device->fd = -1; - } - } - return 0; -} - static struct btrfs_root *open_ctree_broken(int fd, const char *device) { u32 sectorsize; @@ -217,7 +198,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device) out_chunk: free_extent_buffer(fs_info->chunk_root->node); out_devices: - close_all_devices(fs_info); + btrfs_close_devices(fs_info->fs_devices); out_cleanup: extent_io_tree_cleanup(&fs_info->extent_cache); extent_io_tree_cleanup(&fs_info->free_space_cache); diff --git a/disk-io.c b/disk-io.c index 4003636..a8176a5 100644 --- a/disk-io.c +++ b/disk-io.c @@ -35,8 +35,6 @@ #include "utils.h" #include "print-tree.h" -static int close_all_devices(struct btrfs_fs_info *fs_info); - static int check_tree_block(struct btrfs_root *root, struct extent_buffer *buf) { @@ -1028,7 +1026,7 @@ out_chunk: if (fs_info->chunk_root) free_extent_buffer(fs_info->chunk_root->node); out_devices: - close_all_devices(fs_info); + btrfs_close_devices(fs_info->fs_devices); out_cleanup: extent_io_tree_cleanup(&fs_info->extent_cache); extent_io_tree_cleanup(&fs_info->free_space_cache); @@ -1261,30 +1259,6 @@ int write_ctree_super(struct btrfs_trans_handle *trans, return ret; } -static int close_all_devices(struct btrfs_fs_info *fs_info) -{ - struct list_head *list; - struct btrfs_device *device; - - list = &fs_info->fs_devices->devices; - while (!list_empty(list)) { - device = list_entry(list->next, struct btrfs_device, dev_list); - list_del_init(&device->dev_list); - if (device->fd != -1) { - fsync(device->fd); - if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED)) - fprintf(stderr, "Warning, could not drop caches\n"); - close(device->fd); - device->fd = -1; - } - kfree(device->name); - kfree(device->label); - kfree(device); - } - kfree(fs_info->fs_devices); - return 0; -} - static void free_mapping_cache(struct btrfs_fs_info *fs_info) { struct cache_tree *cache_tree = &fs_info->mapping_tree.cache_tree; @@ -1337,7 +1311,7 @@ int close_ctree(struct btrfs_root *root) free(fs_info->log_root_tree); } - close_all_devices(fs_info); + btrfs_close_devices(fs_info->fs_devices); free_mapping_cache(fs_info); extent_io_tree_cleanup(&fs_info->extent_cache); extent_io_tree_cleanup(&fs_info->free_space_cache); diff --git a/volumes.c b/volumes.c index b88385b..0f6a35b 100644 --- a/volumes.c +++ b/volumes.c @@ -163,6 +163,9 @@ again: list_for_each(cur, &fs_devices->devices) { device = list_entry(cur, struct btrfs_device, dev_list); if (device->fd != -1) { + fsync(device->fd); + if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED)) + fprintf(stderr, "Warning, could not drop caches\n"); close(device->fd); device->fd = -1; }
-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html