dm persistent data: stop using dm_bm_unlock_move when shadowing blocks in tm

Stop using dm_bm_unlock_move when shadowing blocks in the transaction
manager as an optimisation and remove the function as it is then no
longer used.

Some code, such as the space maps, keeps using on-disk data structures
from the previous transaction.  It can do this because blocks won't
be reallocated until the subsequent transaction.  Using
dm_bm_unlock_move to copy blocks sounds like a win, but it forces a
synchronous read should the old block be accessed.

Signed-off-by: Joe Thornber <ejt@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>

diff --git a/drivers/md/persistent-data/dm-block-manager.c b/drivers/md/persistent-data/dm-block-manager.c
index 4b5c504f47afb16a74bce66a53f82abebef54f21..ad1712e802f60557e910350371beab89652218f6 100644 (file)
--- a/drivers/md/persistent-data/dm-block-manager.c
+++ b/drivers/md/persistent-data/dm-block-manager.c
@@ -584,22 +584,6 @@ int dm_bm_unlock(struct dm_block *b)
 }
 EXPORT_SYMBOL_GPL(dm_bm_unlock);
 
-int dm_bm_unlock_move(struct dm_block *b, dm_block_t n)
-{
-       struct buffer_aux *aux;
-
-       aux = dm_bufio_get_aux_data(to_buffer(b));
-
-       if (aux->write_locked) {
-               dm_bufio_mark_buffer_dirty(to_buffer(b));
-               bl_up_write(&aux->lock);
-       } else
-               bl_up_read(&aux->lock);
-
-       dm_bufio_release_move(to_buffer(b), n);
-       return 0;
-}
-
 int dm_bm_flush_and_unlock(struct dm_block_manager *bm,
                           struct dm_block *superblock)
 {

diff --git a/drivers/md/persistent-data/dm-block-manager.h b/drivers/md/persistent-data/dm-block-manager.h
index 924833d2dfa69f13944628eb220733a5500cd807..327885566631b87acbffa1bfade4cdc481efe8c8 100644 (file)
--- a/drivers/md/persistent-data/dm-block-manager.h
+++ b/drivers/md/persistent-data/dm-block-manager.h
@@ -96,14 +96,6 @@ int dm_bm_write_lock_zero(struct dm_block_manager *bm, dm_block_t b,
 
 int dm_bm_unlock(struct dm_block *b);
 
-/*
- * An optimisation; we often want to copy a block's contents to a new
- * block.  eg, as part of the shadowing operation.  It's far better for
- * bufio to do this move behind the scenes than hold 2 locks and memcpy the
- * data.
- */
-int dm_bm_unlock_move(struct dm_block *b, dm_block_t n);
-
 /*
  * It's a common idiom to have a superblock that should be committed last.
  *

diff --git a/drivers/md/persistent-data/dm-transaction-manager.c b/drivers/md/persistent-data/dm-transaction-manager.c
index b4f05830af07657a8a7f7434f109ef459c5eb06e..d247a35da3c63030bca417b89482bb8fdf371cd0 100644 (file)
--- a/drivers/md/persistent-data/dm-transaction-manager.c
+++ b/drivers/md/persistent-data/dm-transaction-manager.c
@@ -219,13 +219,24 @@ static int __shadow_block(struct dm_transaction_manager *tm, dm_block_t orig,
        if (r < 0)
                return r;
 
-       r = dm_bm_unlock_move(orig_block, new);
-       if (r < 0) {
+       /*
+        * It would be tempting to use dm_bm_unlock_move here, but some
+        * code, such as the space maps, keeps using the old data structures
+        * secure in the knowledge they won't be changed until the next
+        * transaction.  Using unlock_move would force a synchronous read
+        * since the old block would no longer be in the cache.
+        */
+       r = dm_bm_write_lock_zero(tm->bm, new, v, result);
+       if (r) {
                dm_bm_unlock(orig_block);
                return r;
        }
 
-       return dm_bm_write_lock(tm->bm, new, v, result);
+       memcpy(dm_block_data(*result), dm_block_data(orig_block),
+              dm_bm_block_size(tm->bm));
+
+       dm_bm_unlock(orig_block);
+       return r;
 }
 
 int dm_tm_shadow_block(struct dm_transaction_manager *tm, dm_block_t orig,