On Tue 05-02-13 21:24:33, Zheng Liu wrote: > On Tue, Feb 05, 2013 at 01:08:54PM +0100, Jan Kara wrote: > [snip] > > > After tracking all extent status in status tree, ext4_es_find_extent() > > > returns not only delayed extent, but also written/unwritten extents. So > > > it is possible that next_del == next and its value is not > > > EXT_MAX_BLOCKS. *But* in latest version ext4_es_find_extent() will be > > > changed to only return delayed extent. So the problem will be fixed. > > Ah, now I see. You added the condition checking whether extent is delayed > > only to the newex->ec_start == 0 branch. So if we don't take that branch, > > we could have returned an extent which isn't delayed. > > > > IMHO it is a wrong decision for ext4_es_find_extent() to return only > > delayed extents. That should really return any extent that contains given > > block (or is after it). It is ext4_find_delayed_extent() that should really > > be changed to return only delayed extents as its name suggests... > > I revised the patch series and found that ext4_es_find_extent() is > only used to lookup a delayed extent by the following functions: > > - ext4_find_delalloc_range() > - ext4_find_delayed_extent() > - ext4_seek_data() > - ext4_seek_hole() > > So IMHO the better decision is to rename it to ext4_es_find_delayed_extent() > and let it only return delayed extent. In patch 6/9, a new function > called ext4_es_lookup_extent() is defined to do this job that returns an > extent that contains given block. What do you think? Yes, this works for me. Thanks for looking into it. Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- 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
