On 2/23/11 3:34 AM, Yongqiang Yang wrote: > On Wed, Feb 23, 2011 at 4:59 PM, Yongqiang Yang <xiaoqiangnk@xxxxxxxxx> wrote: >> On Wed, Feb 23, 2011 at 5:18 AM, Chris Mason <chris.mason@xxxxxxxxxx> wrote: >>> [ resend, sorry if this is a dup ] >>> >>> Hi everyone, >>> >>> We've had reports on btrfs that cp is giving us files full of zeros >>> instead of actually copying them. It was tracked down to a bug with >>> the btrfs fiemap implementation where it was returning holes for >>> delalloc ranges. >>> >>> Newer versions of cp are trusting fiemap to tell it where the holes >>> are, which does seem like a pretty neat trick. >>> >>> I decided to give xfs and ext4 a shot with a few tests cases too, xfs >>> passed with all the ones btrfs was getting wrong, and ext4 got the basic >>> delalloc case right. >>> >>> # mkfs.ext4 /dev/xxx >>> # mount /dev/xxx /mnt >>> # dd if=/dev/zero of=/mnt/foo bs=1M count=1 >>> # fiemap-test foo >>> ext: 0 logical: [ 0.. 255] phys: 0.. 255 flags: 0x007 tot: 256 >>> >>> Horray! But once we throw a hole in, things go bad: >>> >>> # mkfs.ext4 /dev/xxx >>> # mount /dev/xxx /mnt >>> # dd if=/dev/zero of=/mnt/foo bs=1M count=1 seek=1 >>> # fiemap-test foo >>> < no output > >> Actually, there is no extent in extent tree now, so >> ext4_ext_walk_space() will pass ext4_ext_fiemap_cb() a variable of >> struct ext4_ext_cache with the requested length. But in >> ext4_ext_fiemap_cb() just the paging contains start block is got >> via find_get_page(), if find_get_page() return null, >> ext4_ext_fiemap_cb() thinks the whole request range is empty and it >> returns request range. >> >> In 1st case, find_get_page() will succeed. >> >> It seems that we should get no. of pages in page cache if >> find_get_page() fails, and correct the range to be returned. > We can call find_get_pages() with nr_pages=1 instead. And we can regulate > the range with page->index if it is not the the paging contains start block. > > >> >> Right? >> >> If right I will send a patch. Your analysis is correct, the way it's working now is pretty broken (my fault I'm afraid) Right now we only look at the first page in a "gap" to see if it's delalloc; we need to search through any dirty pages in the gap, since the first page may be a hole, with delalloc ranges coming later. We need some variant of page cache search, yes. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
