From 86a3aad395340fe6502c1c51b58de30bce4b2e97 Mon Sep 17 00:00:00 2001 From: Tomohiro Kusumi Date: Sun, 18 Jan 2015 01:54:56 +0900 Subject: [PATCH] sys/vfs/hammer: make use of hammer_xlate_to_zone2() This patch makes use of the new macro hammer_xlate_to_zone2() I've added in the previous patch in http://bugs.dragonflybsd.org/issues/2771 I see hammer source does not prefer to use bunch of macros over macros, but rather use raw and/or bitwise operation probably to keep simplicity of the code as well as its design. However a piece of code like (a) makes it difficult to understand what it's trying to do. Using this macro makes it look like (b) which to me makes better sense. (a) base_off = (zone_offset & (~HAMMER_LARGEBLOCK_MASK64 & ~HAMMER_OFF_ZONE_MASK)) | HAMMER_ZONE_RAW_BUFFER; (b) base_off = hammer_xlate_to_zone2(zone_offset & ~HAMMER_LARGEBLOCK_MASK64); Since I've already sent a patch with such a macro to translate zones and the fact that zoneX-to-zone2 translation does have a meaning, I'd like to use that in other functions of blockmap code. There is no logical change here as following unittest shows. ---------- \# cat ./unittest.c \#include \#include \#include /* new macro */ \#define hammer_xlate_to_zone2(offset) \ ((offset & ~HAMMER_OFF_ZONE_MASK) | HAMMER_ZONE_RAW_BUFFER) /* mimic blockmap translation */ static void assert_blockmap_translation(hammer_off_t next_offset) { hammer_off_t base_off1, base_off2; base_off1 = (next_offset & (~HAMMER_LARGEBLOCK_MASK64 & ~HAMMER_OFF_ZONE_MASK)) | HAMMER_ZONE_RAW_BUFFER; base_off2 = hammer_xlate_to_zone2(next_offset & ~HAMMER_LARGEBLOCK_MASK64); printf("next = 0x%08lX\n", next_offset); printf("off1 = 0x%08lX\n", base_off1); printf("off2 = 0x%08lX\n", base_off2); assert(base_off1 == base_off2); assert(HAMMER_ZONE_DECODE(base_off1) == HAMMER_ZONE_RAW_BUFFER_INDEX); assert(HAMMER_ZONE_DECODE(base_off2) == HAMMER_ZONE_RAW_BUFFER_INDEX); } int main(void) { assert_blockmap_translation(0xA011223344556677); assert_blockmap_translation(0x8FFFFFFFFFFFFFFF); printf("success\n"); return 0; } \# gcc -Wall -g ./unittest.c -o unittest \# ./unittest next = 0xA011223344556677 off1 = 0x2011223344000000 off2 = 0x2011223344000000 next = 0x8FFFFFFFFFFFFFFF off1 = 0x2FFFFFFFFF800000 off2 = 0x2FFFFFFFFF800000 success ---------- This patch goes on top of following two patches I've submitted. Sorry that the patch series # is disconnected from the previous two. http://bugs.dragonflybsd.org/issues/2770 [PATCH 1/2] sys/vfs/hammer: make variable names up-to-date with code http://bugs.dragonflybsd.org/issues/2771 [PATCH 2/2] sys/vfs/hammer: make hammer_blockmap_lookup() lightweight --- sys/vfs/hammer/hammer_blockmap.c | 21 ++++++++++----------- sys/vfs/hammer/hammer_ondisk.c | 3 +-- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/sys/vfs/hammer/hammer_blockmap.c b/sys/vfs/hammer/hammer_blockmap.c index 51a1efa..7286360 100644 --- a/sys/vfs/hammer/hammer_blockmap.c +++ b/sys/vfs/hammer/hammer_blockmap.c @@ -282,9 +282,8 @@ again: * The bigblock might be reserved by another zone. If it is reserved * by our zone we may have to move next_offset past the append_off. */ - base_off = (next_offset & - (~HAMMER_LARGEBLOCK_MASK64 & ~HAMMER_OFF_ZONE_MASK)) | - HAMMER_ZONE_RAW_BUFFER; + base_off = hammer_xlate_to_zone2(next_offset & + ~HAMMER_LARGEBLOCK_MASK64); resv = RB_LOOKUP(hammer_res_rb_tree, &hmp->rb_resv_root, base_off); if (resv) { if (resv->zone != zone) { @@ -560,9 +559,8 @@ again: * The bigblock might be reserved by another zone. If it is reserved * by our zone we may have to move next_offset past the append_off. */ - base_off = (next_offset & - (~HAMMER_LARGEBLOCK_MASK64 & ~HAMMER_OFF_ZONE_MASK)) | - HAMMER_ZONE_RAW_BUFFER; + base_off = hammer_xlate_to_zone2(next_offset & + ~HAMMER_LARGEBLOCK_MASK64); resv = RB_LOOKUP(hammer_res_rb_tree, &hmp->rb_resv_root, base_off); if (resv) { if (resv->zone != zone) { @@ -723,9 +721,8 @@ hammer_blockmap_reserve_dedup(hammer_mount_t hmp, int zone, int bytes, goto failed; } - base_off = (zone_offset & - (~HAMMER_LARGEBLOCK_MASK64 & ~HAMMER_OFF_ZONE_MASK)) | - HAMMER_ZONE_RAW_BUFFER; + base_off = hammer_xlate_to_zone2(zone_offset & + ~HAMMER_LARGEBLOCK_MASK64); resv = RB_LOOKUP(hammer_res_rb_tree, &hmp->rb_resv_root, base_off); if (resv) { if (resv->zone != zone) { @@ -1039,7 +1036,8 @@ hammer_blockmap_free(hammer_transaction_t trans, * occuring. */ if (layer2->bytes_free == HAMMER_LARGEBLOCK_SIZE) { - base_off = (zone_offset & (~HAMMER_LARGEBLOCK_MASK64 & ~HAMMER_OFF_ZONE_MASK)) | HAMMER_ZONE_RAW_BUFFER; + base_off = hammer_xlate_to_zone2(zone_offset & + ~HAMMER_LARGEBLOCK_MASK64); hammer_reserve_setdelay_offset(hmp, base_off, zone, layer2); if (layer2->bytes_free == HAMMER_LARGEBLOCK_SIZE) { @@ -1457,7 +1455,8 @@ hammer_blockmap_lookup_verify(hammer_mount_t hmp, hammer_off_t zone_offset, if (*errorp) goto failed; if (layer2->zone == 0) { - base_off = (zone_offset & (~HAMMER_LARGEBLOCK_MASK64 & ~HAMMER_OFF_ZONE_MASK)) | HAMMER_ZONE_RAW_BUFFER; + base_off = hammer_xlate_to_zone2(zone_offset & + ~HAMMER_LARGEBLOCK_MASK64); resv = RB_LOOKUP(hammer_res_rb_tree, &hmp->rb_resv_root, base_off); KKASSERT(resv && resv->zone == zone); diff --git a/sys/vfs/hammer/hammer_ondisk.c b/sys/vfs/hammer/hammer_ondisk.c index 24985fb..876c5fe 100644 --- a/sys/vfs/hammer/hammer_ondisk.c +++ b/sys/vfs/hammer/hammer_ondisk.c @@ -624,8 +624,7 @@ found_aliased: * the mount is rw. */ buffer = RB_LOOKUP(hammer_buf_rb_tree, &hmp->rb_bufs_root, - (buf_offset & ~HAMMER_OFF_ZONE_MASK) | - HAMMER_ZONE_RAW_BUFFER); + hammer_xlate_to_zone2(buf_offset)); if (buffer) { kprintf("HAMMER: recovered aliased %016jx\n", (intmax_t)buf_offset); -- 2.1.2