Re: [PATCH 1/6] btrfs: qgroup: Skeleton to support separate qgroup reservation type

[Jeff Mahoney <jeffm@suse.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1628 bytes --]

On 10/24/17 8:29 AM, Jeff Mahoney wrote:
> On 10/24/17 7:51 AM, Qu Wenruo wrote:
>>
>>
>> On 2017年10月24日 19:00, Nikolay Borisov wrote:
>>> nit: Why not BTRFS_QGROUP_RSV_TYPES_MAX = 2;
>>
>> My original plan is just as the same as yours.
>>
>> However I still remember I did it before and David fixed it by using
>> TYPES, so I follow his naming schema here.
>>
>> Kernel is also using this naming schema else where:
>> d91876496bcf ("btrfs: compress: put variables defined per compress type
>> in struct to make cache friendly")
> 
> The COMPRESS_TYPES pattern isn't the right pattern to follow here.
> That's a special case since there's a _NONE that doesn't have anything
> associated with it, so we don't need to take a slot in the array.
> 
> We also don't care about any of the specific values, just that they
> start at 0.  The BTRFS_COMPRESS_TYPES example also has a
> BTRFS_COMPRESS_LAST item in the enum, which serves the same purpose as
> MAX.  I don't have a strong opinion on the naming, just that we don't
> play games with +1 when handling arrays since, as you say, that's just
> waiting for subtle bugs later.
> 
> enum btrfs_qgroup_rsv_type {
> 	BTRFS_QGROUP_RSV_DATA = 0,
> 	BTRFS_QGROUP_RSV_META,
> 	BTRFS_QGROUP_RSV_LAST,
> };

It turns out that BTRFS_COMPRESS_LAST did exist until very recently:

commit dc2f29212a2648164b054016dc5b948bf0fc92d5
Author: Anand Jain <anand.jain@oracle.com>
Date:   Sun Aug 13 12:02:41 2017 +0800

    btrfs: remove unused BTRFS_COMPRESS_LAST

... so it was there, but just wasn't used.

-Jeff

-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]