From a5247a1c6fc17e6bb7729186f613a5b9883fa983 Mon Sep 17 00:00:00 2001 From: Tomohiro Kusumi Date: Fri, 16 Jan 2015 22:41:18 +0900 Subject: [PATCH 2/2] sys/vfs/hammer: make hammer_blockmap_lookup() lightweight by splitting into lookup and verify This patch makes a core function hammer_blockmap_lookup() lightweight by splitting it into inlined lookup part and verification part. The problem with current hammer_blockmap_lookup() is that this function really does nothing other than flipping zone bits (blockmap offset translation) when hammer_verify_zone is disabled (set to 0) which is default. It should be inlined to eliminate unnecessary overhead. It's fairly expensive to call a function with 3 args only to change upper 4 bits of the 64 bits address to a static value 0x2, considering this function is frequently called by hammer_get_buffer() for blockmap offset translation whenever hammer needs a buffer for i/o (except for undo and walking layer1-2) but the buffer isn't cached on the on-memory rbtree. This patch does the following. 1. Split hammer_blockmap_lookup() into inlined hammer_blockmap_lookup() and hammer_blockmap_lookup_verify() 1-1. hammer_blockmap_lookup() only does bits flipping if hammer_verify_zone is disabled (default) 1-2. hammer_blockmap_lookup() does bits flipping plus blockmap verify by calling hammer_blockmap_lookup_verify() if hammer_veirfy_zone is enabled There is no logical change here, as it only attempts to reduce unnecessary overhead within core routines. Cloning dragonfly git repository on hammer happens to call hammer_blockmap_lookup() nearly 10000 times with around 37000 files on my environment which is not too small. This patch goes on top of a cleanup (and obvious) patch I've sent as [PATCH 1/2] sys/vfs/hammer: make variable names up-to-date with code since they both changes the same part regarding zone_offset arg. --- sys/vfs/hammer/hammer.h | 33 +++++++++++++++++++++++++++++++-- sys/vfs/hammer/hammer_blockmap.c | 24 ++++++------------------ 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/sys/vfs/hammer/hammer.h b/sys/vfs/hammer/hammer.h index 578ac7b..cbc3598 100644 --- a/sys/vfs/hammer/hammer.h +++ b/sys/vfs/hammer/hammer.h @@ -1355,8 +1355,8 @@ int hammer_blockmap_finalize(hammer_transaction_t trans, hammer_off_t zone_offset, int bytes); int hammer_blockmap_getfree(hammer_mount_t hmp, hammer_off_t zone_offset, int *curp, int *errorp); -hammer_off_t hammer_blockmap_lookup(hammer_mount_t hmp, hammer_off_t zone_offset, - int *errorp); +hammer_off_t hammer_blockmap_lookup_verify(hammer_mount_t hmp, + hammer_off_t zone_offset, int *errorp); hammer_off_t hammer_undo_lookup(hammer_mount_t hmp, hammer_off_t zone3_off, int *errorp); int64_t hammer_undo_used(hammer_transaction_t trans); @@ -1613,6 +1613,35 @@ hammer_modify_node_done(hammer_node_t node) } hammer_modify_buffer_done(node->buffer); } + +/* + * Translate a zone address to zone-2 address. + */ +#define hammer_xlate_to_zone2(offset) \ + ((offset & ~HAMMER_OFF_ZONE_MASK) | HAMMER_ZONE_RAW_BUFFER) + +/* + * Lookup a blockmap offset. + */ +static __inline hammer_off_t +hammer_blockmap_lookup(hammer_mount_t hmp, hammer_off_t zone_offset, + int *errorp) +{ + int zone = HAMMER_ZONE_DECODE(zone_offset); + KKASSERT(zone >= HAMMER_ZONE_BTREE_INDEX && zone < HAMMER_MAX_ZONES); + + /* + * We can actually skip blockmap verify by default, + * as normal blockmaps are now direct-mapped onto the freemap + * and so represent zone-2 addresses. + */ + if (hammer_verify_zone == 0) { + *errorp = 0; + return hammer_xlate_to_zone2(zone_offset); + } + + return hammer_blockmap_lookup_verify(hmp, zone_offset, errorp); +} #endif #define hammer_modify_volume_field(trans, vol, field) \ diff --git a/sys/vfs/hammer/hammer_blockmap.c b/sys/vfs/hammer/hammer_blockmap.c index 2049e1b..51a1efa 100644 --- a/sys/vfs/hammer/hammer_blockmap.c +++ b/sys/vfs/hammer/hammer_blockmap.c @@ -1398,11 +1398,11 @@ failed: /* - * Lookup a blockmap offset. + * Lookup a blockmap offset and verify blockmap layers. */ hammer_off_t -hammer_blockmap_lookup(hammer_mount_t hmp, hammer_off_t zone_offset, - int *errorp) +hammer_blockmap_lookup_verify(hammer_mount_t hmp, hammer_off_t zone_offset, + int *errorp) { hammer_volume_t root_volume; hammer_blockmap_t freemap; @@ -1420,19 +1420,7 @@ hammer_blockmap_lookup(hammer_mount_t hmp, hammer_off_t zone_offset, * Calculate the zone-2 offset. */ zone = HAMMER_ZONE_DECODE(zone_offset); - KKASSERT(zone >= HAMMER_ZONE_BTREE_INDEX && zone < HAMMER_MAX_ZONES); - - result_offset = (zone_offset & ~HAMMER_OFF_ZONE_MASK) | - HAMMER_ZONE_RAW_BUFFER; - - /* - * We can actually stop here, normal blockmaps are now direct-mapped - * onto the freemap and so represent zone-2 addresses. - */ - if (hammer_verify_zone == 0) { - *errorp = 0; - return(result_offset); - } + result_offset = hammer_xlate_to_zone2(zone_offset); /* * Validate the allocation zone @@ -1475,7 +1463,7 @@ hammer_blockmap_lookup(hammer_mount_t hmp, hammer_off_t zone_offset, KKASSERT(resv && resv->zone == zone); } else if (layer2->zone != zone) { - panic("hammer_blockmap_lookup: bad zone %d/%d", + panic("hammer_blockmap_lookup_verify: bad zone %d/%d", layer2->zone, zone); } if (layer2->entry_crc != crc32(layer2, HAMMER_LAYER2_CRCSIZE)) { @@ -1490,7 +1478,7 @@ failed: hammer_rel_buffer(buffer, 0); hammer_rel_volume(root_volume, 0); if (hammer_debug_general & 0x0800) { - kprintf("hammer_blockmap_lookup: %016llx -> %016llx\n", + kprintf("hammer_blockmap_lookup_verify: %016llx -> %016llx\n", (long long)zone_offset, (long long)result_offset); } return(result_offset); -- 2.1.2