Re: [PATCH 2/4] page_cache_readahead: remove duplicated code

[Steven Pratt]

I like this one, especially getting rid of the large duplicate comment.  
No functional difference.

Signed-off-by: Steven Pratt <slpratt@xxxxxxxxxxxxx>

Oleg Nesterov wrote:

> Cases "no ahead window" and "crossed into ahead window"
> can be unified.
>
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
>
> --- 2.6.11-rc2/mm/readahead.c~	2005-01-25 14:28:22.000000000 +0300
> +++ 2.6.11-rc2/mm/readahead.c	2005-01-25 15:17:13.000000000 +0300
> @@ -483,43 +483,24 @@ page_cache_readahead(struct address_spac
> 	 * occurence (ie we have an existing window)
> 	 */
>
> -	if (ra->ahead_start == 0) {	 /* no ahead window yet */
> -		ra->ahead_size = get_next_ra_size(ra->size, max, min,
> -						  &ra->flags);
> -		ra->ahead_start = ra->start + ra->size;
> -		block = ((offset + newsize -1) >= ra->ahead_start);
> -		if (!blockable_page_cache_readahead(mapping, filp,
> -		    ra->ahead_start, ra->ahead_size, ra, block)) {
> -			/* A read failure in blocking mode, implies pages are
> -			 * all cached. So we can safely assume we have taken
> -			 * care of all the pages requested in this call. A read
> -			 * failure in non-blocking mode, implies we are reading
> -			 * more pages than requested in this call.  So we safely
> -			 * assume we have taken care of all the pages requested
> -			 * in this call.
> -			 *
> -			 * Just reset the ahead window in case we failed due to
> -			 * congestion.  The ahead window will any way be closed
> -			 * in case we failed due to exessive page cache hits.
> -			 */
> -			ra->ahead_start = 0;
> -			ra->ahead_size = 0;
> -			goto out;
> -		}
> -	}
> 	/*
> -	 * Already have an ahead window, check if we crossed into it.
> +	 * Check if we crossed into ahead window.
> 	 * If so, shift windows and issue a new ahead window.
> 	 * Only return the #pages that are in the current window, so that
> 	 * we get called back on the first page of the ahead window which
> 	 * will allow us to submit more IO.
> 	 */
> 	if ((offset + newsize - 1) >= ra->ahead_start) {
> -		ra->start = ra->ahead_start;
> -		ra->size = ra->ahead_size;
> -		ra->ahead_start = ra->ahead_start + ra->ahead_size;
> -		ra->ahead_size = get_next_ra_size(ra->ahead_size,
> -						  max, min, &ra->flags);
> +		/* Check if we already have an ahead window */
> +		if (ra->ahead_start != 0) {
> +			ra->start = ra->ahead_start;
> +			ra->size = ra->ahead_size;
> +		}
> +
> +		ra->ahead_size = get_next_ra_size(ra->size, max, min,
> +							&ra->flags);
> +		ra->ahead_start = ra->start + ra->size;
> +
> 		block = ((offset + newsize - 1) >= ra->ahead_start);
> 		if (!blockable_page_cache_readahead(mapping, filp,
> 			ra->ahead_start, ra->ahead_size, ra, block)) {
>  

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/