Re: [PATCH 1/5] btrfs: add flags to give an hint to the chunk allocator

[Josef Bacik <josef@toxicpanda.com>]

On Sun, Mar 06, 2022 at 07:14:39PM +0100, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli <kreijack@inwind.it>
> 
> Add the following flags to give an hint about which chunk should be
> allocated in which disk:
> 
> - BTRFS_DEV_ALLOCATION_HINT_DATA_PREFERRED
>   preferred for data chunk, but metadata chunk allowed
> - BTRFS_DEV_ALLOCATION_HINT_METADATA_PREFERRED
>   preferred for metadata chunk, but data chunk allowed
> - BTRFS_DEV_ALLOCATION_HINT_METADATA_ONLY
>   only metadata chunk allowed
> - BTRFS_DEV_ALLOCATION_HINT_DATA_ONLY
>   only data chunk allowed
> 
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---
>  include/uapi/linux/btrfs_tree.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> index b069752a8ecf..e0d842c2e616 100644
> --- a/include/uapi/linux/btrfs_tree.h
> +++ b/include/uapi/linux/btrfs_tree.h
> @@ -389,6 +389,22 @@ struct btrfs_key {
>  	__u64 offset;
>  } __attribute__ ((__packed__));
>  
> +/* dev_item.type */
> +
> +/* btrfs chunk allocation hint */
> +#define BTRFS_DEV_ALLOCATION_HINT_BIT_COUNT	2
> +/* btrfs chunk allocation hint mask */
> +#define BTRFS_DEV_ALLOCATION_HINT_MASK	\
> +	((1 << BTRFS_DEV_ALLOCATION_HINT_BIT_COUNT) - 1)
> +/* preferred data chunk, but metadata chunk allowed */
> +#define BTRFS_DEV_ALLOCATION_HINT_DATA_PREFERRED	(0ULL)
> +/* preferred metadata chunk, but data chunk allowed */
> +#define BTRFS_DEV_ALLOCATION_HINT_METADATA_PREFERRED	(1ULL)
> +/* only metadata chunk are allowed */
> +#define BTRFS_DEV_ALLOCATION_HINT_METADATA_ONLY		(2ULL)
> +/* only data chunk allowed */
> +#define BTRFS_DEV_ALLOCATION_HINT_DATA_ONLY		(3ULL)
> +

Just a style thing here, format it to match everything else, and have the bit
stuff later, as well as newlines between everything since we're commenting.
Also we don't need the () around the values, generally it's best to follow the
style of the surrounding code when in doubt.

I know Dave probably has stronger opinions on this than I do, but I think
something like this would be better.

/*
 * btrfs_dev_item.type values.
 *
 * The _PREFERRED options indicate we prefer to allocate the respective type on
 * this device, but will fall back to allocating any chunk type if we run out of
 * space.  The _ONLY options indicate we will only allocate the given type from
 * this device and no other types.
 */
#define BTRFS_DEV_ALLOCATION_HINT_DATA_PREFERRED	0ULL
#define BTRFS_DEV_ALLOCATION_HINT_METADATA_PREFERRED	1ULL
....

#define BTRFS_DEV_ALLOCATION_HINT_BIT_COUNT	2
#define BTRFS_DEV_ALLOCATION_HINT_MASK		\
	((1 << BTRFS_DEV_ALLOCATION_HINT_BIT_COUNT) - 1)

This will make it a bit clearer and easier to read.  Thanks,

Josef