Thanks Mikulas, I'll test this morning.
- Joe On Tue, Oct 11, 2022 at 8:10 PM Mikulas Patocka <mpato...@redhat.com> wrote: > Hi > > Here I'm sending updated patch 4 that fixes hang on discard. We must not > do the optimization in dm_btree_lookup_next. > > Mikulas > > > From: Mikulas Patocka <mpato...@redhat.com> > > This patch reduces lock contention in btree walks. We modify the > functions init_ro_wpin, exit_ro_spine and ro_step so that they use > dm_bufio_lock_read/dm_bufio_get_unlocked/dm_bufio_unlock_read. If > dm_bm_fast_get_block fails, we fallback to normal locking. > > When doing tests on pmem and fully provisioned thin volume, it improves > thoughput from 286MiB/s to 442MiB/s. > > fio --ioengine=psync --iodepth=1 --rw=randrw --bs=4k --direct=1 > --numjobs=12 --time_based --runtime=10 --group_reporting --name=/dev/vg/thin > before: > READ: bw=286MiB/s > WRITE: bw=286MiB/s > after: > READ: bw=442MiB/s > WRITE: bw=442MiB/s > > Signed-off-by: Mikulas Patocka <mpato...@redhat.com> > > --- > drivers/md/persistent-data/dm-block-manager.c | 32 +++++++++++++++ > drivers/md/persistent-data/dm-block-manager.h | 6 ++ > drivers/md/persistent-data/dm-btree-internal.h | 4 + > drivers/md/persistent-data/dm-btree-spine.c | 41 > ++++++++++++++++++-- > drivers/md/persistent-data/dm-btree.c | 6 +- > drivers/md/persistent-data/dm-transaction-manager.c | 17 ++++++++ > drivers/md/persistent-data/dm-transaction-manager.h | 6 ++ > 7 files changed, 104 insertions(+), 8 deletions(-) > > Index: linux-2.6/drivers/md/persistent-data/dm-block-manager.c > =================================================================== > --- linux-2.6.orig/drivers/md/persistent-data/dm-block-manager.c > +++ linux-2.6/drivers/md/persistent-data/dm-block-manager.c > @@ -601,6 +601,38 @@ void dm_bm_unlock(struct dm_block *b) > } > EXPORT_SYMBOL_GPL(dm_bm_unlock); > > +void dm_bm_fast_lock(struct dm_block_manager *bm) > +{ > + dm_bufio_lock_read(bm->bufio); > +} > + > +void dm_bm_fast_unlock(struct dm_block_manager *bm) > +{ > + dm_bufio_unlock_read(bm->bufio); > +} > + > +int dm_bm_fast_get_block(struct dm_block_manager *bm, > + dm_block_t b, struct dm_block_validator *v, > + struct dm_block **result) > +{ > + int r; > + struct buffer_aux *aux; > + void *p; > + > + p = dm_bufio_get_unlocked(bm->bufio, b, (struct dm_buffer **) > result); > + if (IS_ERR(p)) > + return PTR_ERR(p); > + if (unlikely(!p)) > + return -EWOULDBLOCK; > + > + aux = dm_bufio_get_aux_data(to_buffer(*result)); > + r = dm_bm_validate_buffer(bm, to_buffer(*result), aux, v); > + if (unlikely(r)) > + return r; > + > + return 0; > +} > + > int dm_bm_flush(struct dm_block_manager *bm) > { > if (dm_bm_is_read_only(bm)) > Index: linux-2.6/drivers/md/persistent-data/dm-block-manager.h > =================================================================== > --- linux-2.6.orig/drivers/md/persistent-data/dm-block-manager.h > +++ linux-2.6/drivers/md/persistent-data/dm-block-manager.h > @@ -96,6 +96,12 @@ int dm_bm_write_lock_zero(struct dm_bloc > > void dm_bm_unlock(struct dm_block *b); > > +void dm_bm_fast_lock(struct dm_block_manager *bm); > +void dm_bm_fast_unlock(struct dm_block_manager *bm); > +int dm_bm_fast_get_block(struct dm_block_manager *bm, > + dm_block_t b, struct dm_block_validator *v, > + struct dm_block **result); > + > /* > * It's a common idiom to have a superblock that should be committed last. > * > Index: linux-2.6/drivers/md/persistent-data/dm-btree-internal.h > =================================================================== > --- linux-2.6.orig/drivers/md/persistent-data/dm-btree-internal.h > +++ linux-2.6/drivers/md/persistent-data/dm-btree-internal.h > @@ -64,9 +64,11 @@ struct ro_spine { > struct dm_btree_info *info; > > struct dm_block *node; > + bool fast_locked; > + bool fast_lock_failed; > }; > > -void init_ro_spine(struct ro_spine *s, struct dm_btree_info *info); > +void init_ro_spine(struct ro_spine *s, struct dm_btree_info *info, bool > disable_fast_access); > void exit_ro_spine(struct ro_spine *s); > int ro_step(struct ro_spine *s, dm_block_t new_child); > struct btree_node *ro_node(struct ro_spine *s); > Index: linux-2.6/drivers/md/persistent-data/dm-btree-spine.c > =================================================================== > --- linux-2.6.orig/drivers/md/persistent-data/dm-btree-spine.c > +++ linux-2.6/drivers/md/persistent-data/dm-btree-spine.c > @@ -118,27 +118,60 @@ void unlock_block(struct dm_btree_info * > dm_tm_unlock(info->tm, b); > } > > +static void bn_fast_lock(struct dm_btree_info *info) > +{ > + dm_tm_fast_lock(info->tm); > +} > + > +static void bn_fast_unlock(struct dm_btree_info *info) > +{ > + dm_tm_fast_unlock(info->tm); > +} > + > +static int bn_fast_get_block(struct dm_btree_info *info, dm_block_t b, > + struct dm_block **result) > +{ > + return dm_tm_fast_get_block(info->tm, b, &btree_node_validator, > result); > +} > + > /*----------------------------------------------------------------*/ > > -void init_ro_spine(struct ro_spine *s, struct dm_btree_info *info) > +void init_ro_spine(struct ro_spine *s, struct dm_btree_info *info, bool > disable_fast_access) > { > s->info = info; > s->node = NULL; > + s->fast_locked = false; > + s->fast_lock_failed = disable_fast_access; > } > > void exit_ro_spine(struct ro_spine *s) > { > - if (s->node) > + if (s->fast_locked) > + bn_fast_unlock(s->info); > + else if (s->node) > unlock_block(s->info, s->node); > } > > int ro_step(struct ro_spine *s, dm_block_t new_child) > { > if (s->node) { > - unlock_block(s->info, s->node); > + if (unlikely(!s->fast_locked)) > + unlock_block(s->info, s->node); > s->node = NULL; > } > - > + if (likely(!s->fast_lock_failed)) { > + int r; > + if (!s->fast_locked) { > + bn_fast_lock(s->info); > + s->fast_locked = true; > + } > + r = bn_fast_get_block(s->info, new_child, &s->node); > + if (likely(!r)) > + return 0; > + s->fast_lock_failed = true; > + s->fast_locked = false; > + bn_fast_unlock(s->info); > + } > return bn_read_lock(s->info, new_child, &s->node); > } > > Index: linux-2.6/drivers/md/persistent-data/dm-transaction-manager.c > =================================================================== > --- linux-2.6.orig/drivers/md/persistent-data/dm-transaction-manager.c > +++ linux-2.6/drivers/md/persistent-data/dm-transaction-manager.c > @@ -348,6 +348,23 @@ void dm_tm_unlock(struct dm_transaction_ > } > EXPORT_SYMBOL_GPL(dm_tm_unlock); > > +void dm_tm_fast_lock(struct dm_transaction_manager *tm) > +{ > + dm_bm_fast_lock(tm->is_clone ? tm->real->bm : tm->bm); > +} > + > +void dm_tm_fast_unlock(struct dm_transaction_manager *tm) > +{ > + dm_bm_fast_unlock(tm->is_clone ? tm->real->bm : tm->bm); > +} > + > +int dm_tm_fast_get_block(struct dm_transaction_manager *tm, dm_block_t b, > + struct dm_block_validator *v, > + struct dm_block **blk) > +{ > + return dm_bm_fast_get_block(tm->is_clone ? tm->real->bm : tm->bm, > b, v, blk); > +} > + > void dm_tm_inc(struct dm_transaction_manager *tm, dm_block_t b) > { > /* > Index: linux-2.6/drivers/md/persistent-data/dm-transaction-manager.h > =================================================================== > --- linux-2.6.orig/drivers/md/persistent-data/dm-transaction-manager.h > +++ linux-2.6/drivers/md/persistent-data/dm-transaction-manager.h > @@ -96,6 +96,12 @@ int dm_tm_read_lock(struct dm_transactio > > void dm_tm_unlock(struct dm_transaction_manager *tm, struct dm_block *b); > > +void dm_tm_fast_lock(struct dm_transaction_manager *tm); > +void dm_tm_fast_unlock(struct dm_transaction_manager *tm); > +int dm_tm_fast_get_block(struct dm_transaction_manager *tm, dm_block_t b, > + struct dm_block_validator *v, > + struct dm_block **blk); > + > /* > * Functions for altering the reference count of a block directly. > */ > Index: linux-2.6/drivers/md/persistent-data/dm-btree.c > =================================================================== > --- linux-2.6.orig/drivers/md/persistent-data/dm-btree.c > +++ linux-2.6/drivers/md/persistent-data/dm-btree.c > @@ -377,7 +377,7 @@ int dm_btree_lookup(struct dm_btree_info > __le64 internal_value_le; > struct ro_spine spine; > > - init_ro_spine(&spine, info); > + init_ro_spine(&spine, info, false); > for (level = 0; level < info->levels; level++) { > size_t size; > void *value_p; > @@ -472,7 +472,7 @@ int dm_btree_lookup_next(struct dm_btree > __le64 internal_value_le; > struct ro_spine spine; > > - init_ro_spine(&spine, info); > + init_ro_spine(&spine, info, true); > for (level = 0; level < info->levels - 1u; level++) { > r = btree_lookup_raw(&spine, root, keys[level], > lower_bound, rkey, > @@ -1369,7 +1369,7 @@ static int dm_btree_find_key(struct dm_b > int r = 0, count = 0, level; > struct ro_spine spine; > > - init_ro_spine(&spine, info); > + init_ro_spine(&spine, info, false); > for (level = 0; level < info->levels; level++) { > r = find_key(&spine, root, find_highest, result_keys + > level, > level == info->levels - 1 ? NULL : &root); > >
-- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel