[DragonFlyBSD - Submit #2772] (Closed) [PATCH] sys/vfs/hammer: make use of hammer_xlate_to_zone2()

[bugtracker-admin at leaf.dragonflybsd.org]

Issue #2772 has been updated by dillon.

Status changed from New to Closed

Applied to master in f4fe61c211f76405279f2734eaff5e7285174523.

The only real modification I made was to the hammer_xlate_to_zone2() macro.  The use of 'offset' in the macro body had to be enclosed in parenthesis in order to ensure that no arithmatic reordering occurs if something complex is passed into the macro as its argument (e.g. in some future use case).

Generally speaking I support inline replacements which make the code more clear.  Yes, the reason I don't use them quite as much is precisely because over-use can often hide complexities that need to be understood when reading the higher level code.  But in this case it does make things more clear so it is ok.

-Matt

----------------------------------------
Submit #2772: [PATCH] sys/vfs/hammer: make use of hammer_xlate_to_zone2()
http://bugs.dragonflybsd.org/issues/2772#change-12491

* Author: tkusumi
* Status: Closed
* Priority: Normal
* Assignee: 
* Category: VFS subsystem
* Target version: 
----------------------------------------
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 <stdio.h>
\#include <assert.h>
\#include <vfs/hammer/hammer_disk.h>

/* 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


---Files--------------------------------
0001-sys-vfs-hammer-make-use-of-hammer_xlate_to_zone2.patch (5.85 KB)


-- 
You have received this notification because you have either subscribed to it, or are involved in it.
To change your notification preferences, please click here: http://bugs.dragonflybsd.org/my/account