[dm-devel] [PATCH 1 of 2] DM Snapshot: remove redundant valid test

Mikulas Patocka mpatocka at redhat.com
Thu Sep 24 14:05:06 UTC 2009


On Thu, 24 Sep 2009, Alasdair G Kergon wrote:

> On Wed, Sep 23, 2009 at 10:19:08AM -0500, Jon Brassow wrote:
> > Patch name: dm-snapshot-remove-redundant-valid-test.patch
> > There is another valid test when we take the lock, the previous test can
> > be dropped.
>  I'm going to leave this one for now, until I've seen the performance 
> analysis. - Is the argument here that the gain from not checking it 
> twice while the snapshot is valid outweighs the cost of the extra 
> locking when the snapshot is full and pending I/O is being failed as 
> rapidly as possible and by that time there'll be no other threads 
> fighting for that lock?

The argument is that it is pointless to check the same variable twice. 
This is not about execution performance, this is about programmer 
performance --- everybody who looks at the code starts thinking "oh, why 
is it checked twice, what's hapenning there?". So the need for double 
checking should be either explained or it should be removed.

Performance of I/O on full snapshot is irrelevant, it doesn't matter if 
you return error few tens or hundreds of cycles earlies or later, the data 
will be lost anyway :)

Mikulas

> Alasdair
> 




More information about the dm-devel mailing list