-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
znode_t::z_{unlinked,zn_prefetch} are not boolean #9092
Conversation
module/zfs/zfs_dir.c
Outdated
@@ -423,7 +423,7 @@ zfs_dirlook(znode_t *dzp, char *name, struct inode **ipp, int flags, | |||
if (error == 0) { | |||
*ipp = ZTOI(zp); | |||
zfs_dirent_unlock(dl); | |||
dzp->z_zn_prefetch = B_TRUE; /* enable prefetching */ | |||
dzp->z_zn_prefetch = 1; /* enable prefetching */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it would be worthwhile to convert all of these uint8_t
's to boolean_t
's and to co-locate them. That should save a full uint64_t
per znode_t
which does add up! What do you think of something like this?
diff --git a/include/sys/zfs_znode.h b/include/sys/zfs_znode.h
index d4a3ea7..ef53684 100644
--- a/include/sys/zfs_znode.h
+++ b/include/sys/zfs_znode.h
@@ -192,10 +192,14 @@ typedef struct znode {
krwlock_t z_name_lock; /* "master" lock for dirent locks */
zfs_dirlock_t *z_dirlocks; /* directory entry lock list */
rangelock_t z_rangelock; /* file range locks */
- uint8_t z_unlinked; /* file has been unlinked */
- uint8_t z_atime_dirty; /* atime needs to be synced */
- uint8_t z_zn_prefetch; /* Prefetch znodes? */
- uint8_t z_moved; /* Has this znode been moved? */
+ boolean_t z_unlinked; /* file has been unlinked */
+ boolean_t z_atime_dirty; /* atime needs to be synced */
+ boolean_t z_zn_prefetch; /* Prefetch znodes? */
+ boolean_t z_moved; /* Has this znode been moved? */
+ boolean_t z_is_sa; /* are we native sa? */
+ boolean_t z_is_mapped; /* are we mmap'ed */
+ boolean_t z_is_ctldir; /* are we .zfs entry */
+ boolean_t z_is_stale; /* are we stale due to rollback? */
uint_t z_blksz; /* block size in bytes */
uint_t z_seq; /* modification sequence number */
uint64_t z_mapcnt; /* number of pages mapped to file */
@@ -212,10 +216,6 @@ typedef struct znode {
uint64_t z_projid; /* project ID */
list_node_t z_link_node; /* all znodes in fs link */
sa_handle_t *z_sa_hdl; /* handle to sa data */
- boolean_t z_is_sa; /* are we native sa? */
- boolean_t z_is_mapped; /* are we mmap'ed */
- boolean_t z_is_ctldir; /* are we .zfs entry */
- boolean_t z_is_stale; /* are we stale due to rollback? */
struct inode z_inode; /* generic vfs inode */
} znode_t;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
I also prefer this way, if we want to change znode_t
(increases diff vs other zfs) for non-functional stuff.
Codecov Report
@@ Coverage Diff @@
## master #9092 +/- ##
===========================================
- Coverage 79.32% 66.81% -12.52%
===========================================
Files 400 326 -74
Lines 121640 104364 -17276
===========================================
- Hits 96493 69729 -26764
- Misses 25147 34635 +9488
Continue to review full report at Codecov.
|
Given znode_t is an incore structure, it's more readable to have them as boolean. Also co-locate existing boolean fields with them for space efficiency (expecting 8 booleans to be packed/aligned). Signed-off-by: Tomohiro Kusumi <[email protected]>
Given znode_t is an in-core structure, it's more readable to have them as boolean. Also co-locate existing boolean fields with them for space efficiency (expecting 8 booleans to be packed/aligned). Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Tomohiro Kusumi <[email protected]> Closes openzfs#9092 Conflicts: include/sys/zfs_znode.h module/zfs/zfs_znode.c
Given znode_t is an in-core structure, it's more readable to have them as boolean. Also co-locate existing boolean fields with them for space efficiency (expecting 8 booleans to be packed/aligned). Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Tomohiro Kusumi <[email protected]> Closes openzfs#9092 Conflicts: include/sys/zfs_znode.h module/zfs/zfs_znode.c
Given znode_t is an in-core structure, it's more readable to have them as boolean. Also co-locate existing boolean fields with them for space efficiency (expecting 8 booleans to be packed/aligned). Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Tomohiro Kusumi <[email protected]> Closes #9092 Conflicts: include/sys/zfs_znode.h module/zfs/zfs_znode.c
Motivation and Context
Description
Several boolean-like members in
znode_t
(incore structure) areof
uint8_t
. Change assignments to use 1/0. Majority of them arealready 1/0 with some exceptions fixed in this commit.
How Has This Been Tested?
Types of changes
Checklist:
Signed-off-by
.