>> +#define BTRFS_ROOT_SNAP_RDONLY (1ULL << 0) >> + >> struct btrfs_root_item { >> struct btrfs_inode_item inode; >> __le64 generation; >> @@ -1116,6 +1118,7 @@ struct btrfs_root { >> int defrag_running; >> char *name; >> int in_sysfs; >> + bool readonly; > > Does make sense to store the same information in two places ? > If we have access to root->readonly, we have also access to "root- >> root_item.flags". Because we need the latter, we can get rid of the former. > > > We can replace a test like > > if(root->readonly) > > with > > if(root->root_item.flags & BTRFS_ROOT_SNAP_RDONLY) > > Or better we can create a two macros like: > > #define btrfs_root_readonly(x) ((x)->root_item.flags & BTRFS_ROOT_SNAP_RDONLY) > #define btrfs_root_set_readonly(x, ro) > \ > do{ (x)->root_item.flags = \ > ((x)->root_item.flags & ~BTRFS_ROOT_SNAP_RDONLY) | \ > (ro ? BTRFS_ROOT_SNAP_RDONLY : 0 ); \ > }while(0) > >
Makes sense. (except that inline functions are preferable) > Sorry for to be too late for this kind of suggestion. But I think that this > optimization may help to avoid misalignment between the two variables (see my > reply in the next patch). > [...] -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html