Re: [PATCH 1/1 version2] Extent overlap bugfix in ext4

[Alex Tomas <alex@clusterfs.com>]

>>>>> Mingming Cao (MC) writes:

 MC> Hi, Amit,
 MC> Have you looked at ext4_ext_walk_space()? It calculate the right
 MC> extent length to allocate to avoid overlap before calling block
 MC> allocation callback function is called.

well, it doesn't use cache. 

 MC> What if the start logical block of the exisitng extent is 0 and there
 MC> is overlap? I think that is possible. For example, the exisitng extent
 MC> is (0,100) and you want to insert new extent (0,500), this will
 MC> certainly fail to report the overlap.

I think this situation must not happen in the first place.
get_blocks() should first find existing blocks and return them
(0,100), subsequent get_blocks() should be called for the
following blocks (100,500) and handle them properly.

 MC> Since this overlap check function is called inside
 MC> ext4_ext_insert_extent(), I think this function should check for all
 MC> kinds of overlaps. Here you only check if the new extent is overlap
 MC> with the next extent. Looking at ext4_ext_walk_space(), there are
 MC> total three kinds of overlaps:
 MC> 1) righ port of new extent overlap with path->p_ext,
 MC> 2) left port of new extent overlap with path->p_ext
 MC> 2) right port of new extent overlap with next extent

 MC> I think we are almost repeating the same logic in
 MC> ext4_ext_walk_space() here.

I tend to agree.

thanks, Alex