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

[Andrew Morton]

Alasdair G Kergon <agk@xxxxxxxxxx> 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?
-
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/