Re: [RFC][V8][PATCH 0/5] btrfs: allocation_hint mode

From: Goffredo Baroncelli <kreijack@libero.it>
To: Josef Bacik <josef@toxicpanda.com>
Cc: David Sterba <dsterba@suse.cz>,
	Sinnamohideen Shafeeq <shafeeqs@panasas.com>,
	Paul Jones <paul@pauljones.id.au>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	Zygo Blaxell <ce3g8jdj@umail.furryterror.org>
Subject: Re: [RFC][V8][PATCH 0/5] btrfs: allocation_hint mode
Date: Tue, 14 Dec 2021 19:53:34 +0100	[thread overview]
Message-ID: <97c118df-e7ec-1ef5-8362-e0ecc949db35@libero.it> (raw)
In-Reply-To: <Ybe34gfrxl8O89PZ@localhost.localdomain>

On 12/13/21 22:15, Josef Bacik wrote:
> On Mon, Dec 13, 2021 at 08:54:24PM +0100, Goffredo Baroncelli wrote:
>> Gentle ping :-)
>>
>> Are there anyone of the mains developer interested in supporting this patch ?
>>
>> I am open to improve it if required.
>>
> 
> Sorry I missed this go by.  I like the interface, we don't have a use for
> device->type yet, so this fits nicely.
> 
> I don't see the btrfs-progs patches in my inbox, and these don't apply, so
> you'll definitely need to refresh for a proper review, but looking at these
> patches they seem sane enough, and I like the interface.  I'd like to hear
> Zygo's opinion as well.
> 
> If we're going to use device->type for this, and since we don't have a user of
> device->type, I'd also like you to go ahead and re-name ->type to
> ->allocation_policy, that way it's clear what we're using it for now.

The allocation policy requires only few bits (3 or 4); instead "type" is 64 bit wide.
We can allocate more bits for future extension, but I don't think that it is necessary
to allocate all the bits to the "allocation_policy".

This to say that renaming the field as allocation_policy is too restrictive if in future
we will use the other bits for another purposes.

I don't like the terms "type". May be flags, is it better ?

> 
> I'd also like some xfstests to validate the behavior so we're sure we're testing
> this.  I'd want 1 test to just test the mechanics, like mkfs with different
> policies and validate they're set right, change policies, add/remove disks with
> different policies.
> 
> Then a second test to do something like fsstress with each set of allocation
> policies to validate that we did actually allocate from the correct disks.  For
> this please also test with compression on to make sure the test validation works
> for both normal allocation and compression (ie it doesn't assume writing 5gib of
> data == 5 gib of data usage, as compression chould give you a different value).
> 
> With that in place I think this is the correct way to implement this feature.
> Thanks,

Ok

> 
> Josef

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5