[Ocfs2-tools-devel] [PATCH 1/2] o2image: allocate bitmap blocks only for metadata blocks when backing-up

[Joseph Qi]

You malloc map array when marking bitmap and check if it has been
allocated around. This looks a bit weird.

Thanks,
Joseph

On 18/4/8 09:36, piaojun wrote:
> Allocate bitmap blocks for all the filesystem blocks will cost lots of
> memory, e.g. 64T LUN will cost 2G memory. And that will cause OOM when
> multi-o2image processes run at the same time. We could allocate bitmap
> blocks only for metadata blocks to save lots of memory.
> 
> Signed-off-by: Jun Piao <piaojun at huawei.com>
> ---
>  include/ocfs2/image.h |  1 -
>  libocfs2/image.c      | 66 ++++++++-------------------------------------------
>  o2image/o2image.c     | 15 ++++++++----
>  3 files changed, 20 insertions(+), 62 deletions(-)
> 
> diff --git a/include/ocfs2/image.h b/include/ocfs2/image.h
> index 1acc191..ee4acbc 100644
> --- a/include/ocfs2/image.h
> +++ b/include/ocfs2/image.h
> @@ -77,7 +77,6 @@ struct ocfs2_image_hdr {
>   */
>  struct _ocfs2_image_bitmap_arr {
>  	uint64_t	arr_set_bit_cnt;
> -	char		*arr_self;
>  	char    	*arr_map;
>  };
>  typedef struct _ocfs2_image_bitmap_arr ocfs2_image_bitmap_arr;
> diff --git a/libocfs2/image.c b/libocfs2/image.c
> index 872bdb3..68f4fda 100644
> --- a/libocfs2/image.c
> +++ b/libocfs2/image.c
> @@ -70,8 +70,8 @@ void ocfs2_image_free_bitmap(ocfs2_filesys *ofs)
>  		return;
> 
>  	for (i=0; i<ost->ost_bmpblks; i++)
> -		if (ost->ost_bmparr[i].arr_self)
> -			ocfs2_free(&ost->ost_bmparr[i].arr_self);
> +		if (ost->ost_bmparr[i].arr_map)
> +			ocfs2_free(&ost->ost_bmparr[i].arr_map);
> 
>  	if (ost->ost_bmparr)
>  		ocfs2_free(&ost->ost_bmparr);
> @@ -85,11 +85,9 @@ void ocfs2_image_free_bitmap(ocfs2_filesys *ofs)
>   */
>  errcode_t ocfs2_image_alloc_bitmap(ocfs2_filesys *ofs)
>  {
> -	uint64_t blks, allocsize, leftsize;
> +	uint64_t blks;
>  	struct ocfs2_image_state *ost = ofs->ost;
> -	int indx, i, n;
>  	errcode_t ret;
> -	char *buf;
> 
>  	ost->ost_bmpblks =
>  		((ost->ost_fsblkcnt - 1) / (OCFS2_IMAGE_BITS_IN_BLOCK)) + 1;
> @@ -99,56 +97,6 @@ errcode_t ocfs2_image_alloc_bitmap(ocfs2_filesys *ofs)
>  	/* allocate memory for an array to track bitmap blocks */
>  	ret = ocfs2_malloc0((blks * sizeof(ocfs2_image_bitmap_arr)),
>  			    &ost->ost_bmparr);
> -	if (ret)
> -		return ret;
> -
> -	allocsize = blks * OCFS2_IMAGE_BITMAP_BLOCKSIZE;
> -	leftsize = allocsize;
> -	indx = 0;
> -
> -	/* allocate bitmap blocks and assign blocks to above array */
> -	while (leftsize) {
> -		ret = ocfs2_malloc_blocks(ofs->fs_io,
> -					  allocsize/io_get_blksize(ofs->fs_io),
> -					  &buf);
> -		if (ret && (ret != -ENOMEM))
> -			goto out;
> -
> -		if (ret == -ENOMEM) {
> -			if (allocsize == OCFS2_IMAGE_BITMAP_BLOCKSIZE)
> -				goto out;
> -			allocsize >>= 1;
> -			if (allocsize % OCFS2_IMAGE_BITMAP_BLOCKSIZE) {
> -				allocsize /= OCFS2_IMAGE_BITMAP_BLOCKSIZE;
> -				allocsize *= OCFS2_IMAGE_BITMAP_BLOCKSIZE;
> -			}
> -			continue;
> -		}
> -
> -		n = allocsize / OCFS2_IMAGE_BITMAP_BLOCKSIZE;
> -		for (i = 0; i < n; i++) {
> -			ost->ost_bmparr[indx].arr_set_bit_cnt = 0;
> -			ost->ost_bmparr[indx].arr_map =
> -				((char *)buf + (i *
> -						OCFS2_IMAGE_BITMAP_BLOCKSIZE));
> -
> -			/* remember buf address to free it later */
> -			if (!i)
> -				ost->ost_bmparr[indx].arr_self = buf;
> -			indx++;
> -		}
> -		leftsize -= allocsize;
> -		if (leftsize <= allocsize)
> -			allocsize = leftsize;
> -	}
> -out:
> -	/* If allocation failed free and return error */
> -	if (leftsize) {
> -		for (i = 0; i < indx; i++)
> -			if (ost->ost_bmparr[i].arr_self)
> -				ocfs2_free(&ost->ost_bmparr[i].arr_self);
> -		ocfs2_free(&ost->ost_bmparr);
> -	}
> 
>  	return ret;
>  }
> @@ -251,6 +199,11 @@ void ocfs2_image_mark_bitmap(ocfs2_filesys *ofs, uint64_t blkno)
>  	bit = blkno % OCFS2_IMAGE_BITS_IN_BLOCK;
>  	bitmap_blk = blkno / OCFS2_IMAGE_BITS_IN_BLOCK;
> 
> +	if (!ost->ost_bmparr[bitmap_blk].arr_map) {
> +		ocfs2_malloc0(ost->ost_bmpblksz,
> +				   &ost->ost_bmparr[bitmap_blk].arr_map);
> +	}
> +
>  	ocfs2_set_bit(bit, ost->ost_bmparr[bitmap_blk].arr_map);
>  }
> 
> @@ -263,7 +216,8 @@ int ocfs2_image_test_bit(ocfs2_filesys *ofs, uint64_t blkno)
>  	bit = blkno % OCFS2_IMAGE_BITS_IN_BLOCK;
>  	bitmap_blk = blkno / OCFS2_IMAGE_BITS_IN_BLOCK;
> 
> -	if (ocfs2_test_bit(bit, ost->ost_bmparr[bitmap_blk].arr_map))
> +	if (ost->ost_bmparr[bitmap_blk].arr_map &&
> +			   ocfs2_test_bit(bit, ost->ost_bmparr[bitmap_blk].arr_map))
>  		return 1;
>  	else
>  		return 0;
> diff --git a/o2image/o2image.c b/o2image/o2image.c
> index 1dd16ed..15e0d2d 100644
> --- a/o2image/o2image.c
> +++ b/o2image/o2image.c
> @@ -602,8 +602,11 @@ static errcode_t write_image_file(ocfs2_filesys *ofs, int fd)
>  	}
>  	/* write bitmap blocks at the end */
>  	for(blk = 0; blk < ost->ost_bmpblks; blk++) {
> -		bytes = write(fd, ost->ost_bmparr[blk].arr_map,
> -			      ost->ost_bmpblksz);
> +		if (ost->ost_bmparr[blk].arr_map)
> +			bytes = write(fd, ost->ost_bmparr[blk].arr_map,
> +				      ost->ost_bmpblksz);
> +		else
> +			lseek(fd, ost->ost_bmpblksz, SEEK_CUR);
>  		if ((bytes == -1) || (bytes < ost->ost_bmpblksz)) {
>  			com_err(program_name, errno, "error writing bitmap "
>  				"blk %"PRIu64" bytes %u", blk, bytes);
> @@ -637,9 +640,11 @@ static errcode_t scan_raw_disk(ocfs2_filesys *ofs)
>  	/* update set_bit_cnt for future use */
>  	for (i = 0; i < ost->ost_bmpblks; i++) {
>  		ost->ost_bmparr[i].arr_set_bit_cnt = bits_set;
> -		for (j = 0; j < (ost->ost_bmpblksz * 8); j++)
> -			if (ocfs2_test_bit(j, ost->ost_bmparr[i].arr_map))
> -				bits_set++;
> +		if (ost->ost_bmparr[i].arr_map) {
> +			for (j = 0; j < (ost->ost_bmpblksz * 8); j++)
> +				if (ocfs2_test_bit(j, ost->ost_bmparr[i].arr_map))
> +					bits_set++;
> +		}
>  	}
> 
>  out:
>