Re: [dm-devel] [PATCH 3/4] [block-manager] remove spurious decrement of write_lock_pending in the case of a recycled block.

[Mikulas Patocka <mpatocka redhat com>]

I think this is not correct.

The problem here is that the block may have been recycled and the newly 
created block may have the same block number as the old block.

If b->where != block, we know that the block was recycled.
If b->where == block, the block may have been recycled or not and we 
don't know.


I think the correct solution could be: make write_lock_pending a boolean 
variable, not a counter.

Set write_lock_pending inside __wait_block when we are about to wait (the 
block may have been recycled each time we waited, so we need to set it 
each time we are going to wait)
Clear write_lock_pending when __wait_unlocked exits.

If we make it a boolean variable, double clearing makes no harm.

Mikulas

On Tue, 2 Aug 2011, Joe Thornber wrote:

> ---
>  drivers/md/persistent-data/dm-block-manager.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/md/persistent-data/dm-block-manager.c b/drivers/md/persistent-data/dm-block-manager.c
> index b68be88..d27ab6e 100644
> --- a/drivers/md/persistent-data/dm-block-manager.c
> +++ b/drivers/md/persistent-data/dm-block-manager.c
> @@ -756,9 +756,15 @@ retry:
>  
>  				b->write_lock_pending++;
>  				__wait_unlocked(b, &flags);
> -				b->write_lock_pending--;
>  				if (b->where != block)
> +					/*
> +					 * Recycled blocks have their
> +					 * write_lock_pending count reset
> +					 * to zero, so no need to undo the
> +					 * above increment.
> +					 */
>  					goto retry;
> +				b->write_lock_pending--;
>  			}
>  			break;
>  		}
> -- 
> 1.7.4.1
>