Re: [PATCH] xfstests:Make 225 compare map and fiemap at each block.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/18/2011 09:14 AM, Yongqiang Yang wrote:
> Hi All,
> 
> Due to my carelessness, I induced a ugly patch to ext4's fiemap, but
> 225 could not find it.  So I looked into the 225 and could not figure out
> logic in compare_map_and_fiemap(), which seemed to mixed extents with
> blocks.  Then I made 225 compare map and fiemap at each block, the new
> 225 can find another bug in ext4's fiemap.  After the new 225 is accepted,
> I will send out the patch for ext4's fiemap.
> 
> The new 225 works well on ext3 and ext4 with both 1K and 4K block. However,
> it report fiemap error on xfs with 4K block.  My working tree is 2.6.39-rc3
> pulled from Ted's tree. The error message is as follows.
> 
>  QA output created by 225
>  fiemap run without preallocation, with sync
> +map is 'DDHDHHDHHDHDDHDDHDDHHDHDDHDDDDDDHHDDDHHHHDHDDDDDDDDHDDHHHDDDHDDHHD
> DDDDDHHHHHHDDHHHHHDHDHDHDDDHDDHD'
> +logical: [       0..      15] phys:       12..      27 flags: 0x000 tot: 16
> +logical: [      17..      31] phys:       29..      43 flags: 0x000 tot: 15
> +logical: [      34..      63] phys:       46..      75 flags: 0x000 tot: 30
> +logical: [      65..      95] phys:       77..     107 flags: 0x001 tot: 31
> +Problem comparing fiemap and map
>  fiemap run without preallocation or sync
> +map is 'DDHDHHDHHDHDDHDDHDDHHDHDDHDDDDDDHHDDDHHHHDHDDDDDDDDHDDHHHDDDHDDHHDD
> DDDDHHHHHHDDHHHHHDHDHDHDDDHDDHD'
> +logical: [       0..      15] phys:        0..      15 flags: 0x006 tot: 16
> +Problem comparing fiemap and map
> Ran: 225
> Failures: 225
> Failed 1 of 1 tests
> 
> I am not sure this is a bug in new 225 or xfs.
> 
> Yongqiang.
> 
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
> ---
>  src/fiemap-tester.c |  223 ++++++++++++++++++++++++++++----------------------
>  1 files changed, 125 insertions(+), 98 deletions(-)
> 
> diff --git a/src/fiemap-tester.c b/src/fiemap-tester.c
> index 1663f84..99bb5ce 100644
> --- a/src/fiemap-tester.c
> +++ b/src/fiemap-tester.c
> @@ -14,6 +14,9 @@
>   * You should have received a copy of the GNU General Public License
>   * along with this program; if not, write the Free Software Foundation,
>   * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + * Compare map and fiemap at each block,
> + * Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>, 2011
>   */
>  
>  #include <stdio.h>
> @@ -57,7 +60,7 @@ generate_file_mapping(int blocks, int prealloc)
>  	int num_types = 2, cur_block = 0;
>  	int i = 0;
>  
> -	map = malloc(sizeof(char) * blocks);
> +	map = malloc(sizeof(char) * (blocks + 1));
>  	if (!map)
>  		return NULL;
>  
> @@ -80,7 +83,8 @@ generate_file_mapping(int blocks, int prealloc)
>  		}
>  		cur_block++;
>  	}
> -
> +	

You added whitespace here.

> +	map[blocks] = 0;
>  	return map;
>  }
>  
> @@ -247,55 +251,36 @@ check_flags(struct fiemap *fiemap, int blocksize)
>  }
>  
>  static int
> -check_data(struct fiemap *fiemap, __u64 logical_offset, int blocksize,
> +check_data(struct fiemap_extent * extent ,  __u64 logical_offset, int blocksize,
>  	   int last, int prealloc)
>  {
> -	struct fiemap_extent *extent;
> -	__u64 orig_offset = logical_offset;
> -	int c, found = 0;
> -
> -	for (c = 0; c < fiemap->fm_mapped_extents; c++) {
> -		__u64 start, end;
> -		extent = &fiemap->fm_extents[c];
> -
> -		start = extent->fe_logical;
> -		end = extent->fe_logical + extent->fe_length;
> -
> -		if (logical_offset > end)
> -			continue;
> -
> -		if (logical_offset + blocksize < start)
> -			break;
> -
> -		if (logical_offset >= start &&
> -		    logical_offset < end) {
> -			if (prealloc &&
> -			    !(extent->fe_flags & FIEMAP_EXTENT_UNWRITTEN)) {
> -				printf("ERROR: preallocated extent is not "
> -				       "marked with FIEMAP_EXTENT_UNWRITTEN: "
> -				       "%llu\n",
> -				       (unsigned long long)
> -				       (start / blocksize));
> -				return -1;
> -			}
> -
> -			if (logical_offset + blocksize > end) {
> -				logical_offset = end+1;
> -				continue;
> -			} else {
> -				found = 1;
> -				break;
> -			}
> +	int found = 0;
> +	__u64 start, end;
> +
> +	start = extent->fe_logical;
> +	end = extent->fe_logical + extent->fe_length;
> +
> +	if (logical_offset >= start &&
> +	    logical_offset < end) {
> +		if (prealloc &&
> +		    !(extent->fe_flags & FIEMAP_EXTENT_UNWRITTEN)) {
> +			printf("ERROR: preallocated extent is not "
> +			       "marked with FIEMAP_EXTENT_UNWRITTEN: "
> +			       "%llu\n",
> +			       (unsigned long long)
> +			       (start / blocksize));
> +			return -1;
>  		}
> +		found = 1;
>  	}
> -
> +	

You added whitespace here.

>  	if (!found) {
>  		printf("ERROR: couldn't find extent at %llu\n",
> -		       (unsigned long long)(orig_offset / blocksize));
> +		       (unsigned long long)(logical_offset / blocksize));
>  	} else if (last &&
> -		   !(fiemap->fm_extents[c].fe_flags & FIEMAP_EXTENT_LAST)) {
> +		   !(extent->fe_flags & FIEMAP_EXTENT_LAST)) {
>  		printf("ERROR: last extent not marked as last: %llu\n",
> -		       (unsigned long long)(orig_offset / blocksize));
> +		       (unsigned long long)(logical_offset / blocksize));
>  		found = 0;
>  	}
>  
> @@ -370,37 +355,26 @@ check_weird_fs_hole(int fd, __u64 logical_offset, int blocksize)
>  }
>  
>  static int
> -check_hole(struct fiemap *fiemap, int fd, __u64 logical_offset, int blocksize)
> +check_hole(struct fiemap_extent *extent, int fd, __u64 logical_offset, int blocksize)
>  {
> -	struct fiemap_extent *extent;
> -	int c;
> +	__u64 start, end;
>  
> -	for (c = 0; c < fiemap->fm_mapped_extents; c++) {
> -		__u64 start, end;
> -		extent = &fiemap->fm_extents[c];
> +	start = extent->fe_logical;
> +	end = extent->fe_logical + extent->fe_length;
>  
> -		start = extent->fe_logical;
> -		end = extent->fe_logical + extent->fe_length;
> +	if (logical_offset >= start &&
> +	    logical_offset < end) {
>  
> -		if (logical_offset > end)
> -			continue;
> -		if (logical_offset + blocksize < start)
> -			break;
> +		if (check_weird_fs_hole(fd, logical_offset,
> +					blocksize) == 0)
> +			return 0;
>  
> -		if (logical_offset >= start &&
> -		    logical_offset < end) {
> -
> -			if (check_weird_fs_hole(fd, logical_offset,
> -						blocksize) == 0)
> -				break;
> -
> -			printf("ERROR: found an allocated extent where a hole "
> -			       "should be: %llu\n",
> -			       (unsigned long long)(start / blocksize));
> -			return -1;
> -		}
> +		printf("ERROR: found an allocated extent where a hole "
> +		       "should be: %llu\n",
> +		       (unsigned long long)(start / blocksize));
> +		return -1;
>  	}
> -
> +	

More whitespace.

>  	return 0;
>  }
>  
> @@ -423,9 +397,11 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize, int syncfil
>  {
>  	struct fiemap *fiemap;
>  	char *fiebuf;
> -	int blocks_to_map, ret, cur_extent = 0, last_data;
> +	int blocks_to_map, ret, last_data = -1;
>  	__u64 map_start, map_length;
>  	int i, c;
> +	int cur_block = 0;
> +	int last_found = 0;
>  
>  	if (query_fiemap_count(fd, blocks, blocksize) < 0)
>  		return -1;
> @@ -451,8 +427,11 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize, int syncfil
>  	fiemap->fm_extent_count = blocks_to_map;
>  	fiemap->fm_mapped_extents = 0;
>  
> +	/* check fiemap by looking at each block. */
>  	do {
> -		fiemap->fm_start = map_start;
> +		int nr_extents;
> +
> +		fiemap->fm_start = cur_block * blocksize;
>  		fiemap->fm_length = map_length;
>  
>  		ret = ioctl(fd, FS_IOC_FIEMAP, (unsigned long)fiemap);
> @@ -465,45 +444,93 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize, int syncfil
>  		if (check_flags(fiemap, blocksize))
>  			goto error;
>  
> -		for (i = cur_extent, c = 1; i < blocks; i++, c++) {
> -			__u64 logical_offset = i * blocksize;
> +		nr_extents = fiemap->fm_mapped_extents;
> +		if (nr_extents == 0) {
> +			int block = cur_block + (map_length - 1)/ blocksize;
> +			for (; cur_block <= block && cur_block < blocks; cur_block++) {
> +				/* check hole */
> +				if (map[cur_block] != 'H') {
> +					printf("ERROR: map[%d] should not be "
> +					       "a hole\n", cur_block);
> +					goto error;
> +				}
> +			}
> +			continue;
> +		}
>  
> -			if (c > fiemap->fm_mapped_extents) {
> -				i++;
> -				break;
> +		for (c = 0; c < nr_extents; c++) {
> +			__u64 offset;
> +			int block;
> +			struct fiemap_extent *extent;
> +
> +			if (last_found) {
> +				printf("ERROR: there is extent after"
> +				       "the last extent\n");
> +				goto error;
>  			}
>  
> -			switch (map[i]) {
> -			case 'D':
> -				if (check_data(fiemap, logical_offset,
> -					       blocksize, last_data == i, 0))
> -					goto error;
> -				break;
> -			case 'H':
> -				if (check_hole(fiemap, fd, logical_offset,
> -					       blocksize))
> -					goto error;
> -				break;
> -			case 'P':
> -				if (check_data(fiemap, logical_offset,
> -					       blocksize, last_data == i, 1))
> +			extent = &fiemap->fm_extents[c];
> +			offset = extent->fe_logical;
> +			block = offset / blocksize;
> +
> +			/* check hole. */
> +			for (; cur_block < block; cur_block++) {
> +				if (map[cur_block] != 'H') {
> +					printf("ERROR: map[%d] should not be "
> +					       "a hole\n", cur_block);
>  					goto error;
> -				break;
> -			default:
> -				printf("ERROR: weird value in map: %c\n",
> -				       map[i]);
> +				}
> +			}
> +
> +			offset = extent->fe_logical + extent->fe_length;
> +			block = offset / blocksize;
> +
> +			if (block > blocks) {
> +				printf("ERROR: there are extents beyond EOF\n");
>  				goto error;
>  			}
> +
> +			/* check data */
> +			for (; cur_block < block; cur_block++) {
> +				offset = (__u64)cur_block * blocksize;
> +				last_found = (last_data == cur_block);
> +				switch (map[cur_block]) {
> +				case 'D':
> +					if (check_data(extent, offset,
> +						       blocksize, last_found, 0))
> +						goto error;
> +					break;
> +				case 'H':
> +					if (check_hole(extent, fd, offset,
> +						       blocksize));

Kill the ;, this is what's making xfs fail.  Thanks,

Josef

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs


[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux