>> +#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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
