On Tue, Oct 08, 2013 at 01:31:57PM -0400, Josef Bacik wrote:
> Liu fixed part of this problem and unfortunately I steered him in slightly the
> wrong direction and so didn't completely fix the problem.  The problem is we
> limit the size of the delalloc range we are looking for to max bytes and then 
> we
> try to lock that range.  If we fail to lock the pages in that range we will
> shrink the max bytes to a single page and re loop.  However if our first page 
> is
> inside of the delalloc range then we will end up limiting the end of the range
> to a period before our first page.  This is illustrated below
> 
> [0 -------- delalloc range --------- 256mb]
>                                   [page]
> 
> So find_delalloc_range will return with delalloc_start as 0 and end as 128mb,
> and then we will notice that delalloc_start < *start and adjust it up, but not
> adjust delalloc_end up, so things go sideways.  To fix this we need to not 
> limit

This makes me more confused...
so do you mean that 'delalloc_end < delalloc_start' leads to the not locked 
pages?

-liubo
thanks,

> the max bytes in find_delalloc_range, but in find_lock_delalloc_range and that
> way we don't end up with this confusion.  Thanks,
> 
> Signed-off-by: Josef Bacik <[email protected]>
> ---
>  fs/btrfs/extent_io.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index b48942f..2bf6f46 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -1491,10 +1491,8 @@ static noinline u64 find_delalloc_range(struct 
> extent_io_tree *tree,
>               cur_start = state->end + 1;
>               node = rb_next(node);
>               total_bytes += state->end - state->start + 1;
> -             if (total_bytes >= max_bytes) {
> -                     *end = *start + max_bytes - 1;
> +             if (total_bytes >= max_bytes)
>                       break;
> -             }
>               if (!node)
>                       break;
>       }
> @@ -1636,10 +1634,9 @@ again:
>  
>       /*
>        * make sure to limit the number of pages we try to lock down
> -      * if we're looping.
>        */
> -     if (delalloc_end + 1 - delalloc_start > max_bytes && loops)
> -             delalloc_end = delalloc_start + PAGE_CACHE_SIZE - 1;
> +     if (delalloc_end + 1 - delalloc_start > max_bytes)
> +             delalloc_end = delalloc_start + max_bytes - 1;
>  
>       /* step two, lock all the pages after the page that has start */
>       ret = lock_delalloc_pages(inode, locked_page,
> @@ -1650,8 +1647,7 @@ again:
>                */
>               free_extent_state(cached_state);
>               if (!loops) {
> -                     unsigned long offset = (*start) & (PAGE_CACHE_SIZE - 1);
> -                     max_bytes = PAGE_CACHE_SIZE - offset;
> +                     max_bytes = PAGE_CACHE_SIZE;
>                       loops = 1;
>                       goto again;
>               } else {
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to