Re: optimize one of the cache flushes

[Mike Snitzer <snitzer@redhat.com>]

On Wed, Jul 07 2010 at  6:22pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Hi
> 
> This patch removes the second cache flush if discard isn't supported. 
> The first flush is hard to bypass, so it's not worth doing it.
> 
> Mikulas
> 
> ---
> 
> Don't do the second flush if the request isn't supported.
> 
> If the request fails with -EOPNOTSUPP, don't perform the second flush.
> This can happen with discard+barrier requests. If the device doesn't support
> discard, there would be two useless SYNCHRONIZE CACHE commands.
> 
> The first dm_flush cannot be so easily optimized out, so we leave it there.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  drivers/md/dm.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6.35-rc3-fast/drivers/md/dm.c
> ===================================================================
> --- linux-2.6.35-rc3-fast.orig/drivers/md/dm.c	2010-07-08 00:11:05.000000000 +0200
> +++ linux-2.6.35-rc3-fast/drivers/md/dm.c	2010-07-08 00:12:02.000000000 +0200
> @@ -2365,7 +2365,12 @@ static void process_barrier(struct mappe
>  
>  	if (!bio_empty_barrier(bio)) {
>  		__split_and_process_bio(md, bio);
> -		dm_flush(md);
> +		/*
> +		 * If the request isn't supported, don't waste time with
> +		 * the second flush.
> +		 */
> +		if (md->barrier_error != -EOPNOTSUPP)
> +			dm_flush(md);
>  	}
>  
>  	if (md->barrier_error != DM_ENDIO_REQUEUE)


Doesn't store_barrier_error just record the result of the first empty
barrier (not the -EOPNOTSUPP result of the unsupported discard)?

I'm missing how this change helps avoid the 2nd barrier for the
-EOPNOTSUPP discard case.

... And my testing shows that it doesn't.

Mike