Re: [PATCH] device-mapper snapshot: metadata reading separation

[Andrew Morton <akpm@osdl.org>]

Alasdair G Kergon <agk@redhat.com> wrote:
>
> +static void read_snapshot_metadata(struct dm_snapshot *s)
>  +{
>  +	if (s->have_metadata)
>  +		return;
>  +
>  +	if (s->store.read_metadata(&s->store)) {
>  +		down_write(&s->lock);
>  +		s->valid = 0;
>  +		up_write(&s->lock);
>  +	}
>  +
>  +	s->have_metadata = 1;
>  +}
>  +

I always get suspicious when I see a lock around a plain assignment. 
Sometimes it's legitimate, usually when some other user of the LHS expects
its value to be stable across an entire locked region - it is read multiple
times and those reads are expected to return the same thing.  (I can't
think of any other case).

Maybe this is such a case?