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

From: "Sinnamohideen, Shafeeq" <shafeeqs@panasas.com>
To: Josef Bacik <josef@toxicpanda.com>,
	"kreijack@inwind.it" <kreijack@inwind.it>
Cc: David Sterba <dsterba@suse.cz>, Paul Jones <paul@pauljones.id.au>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	Zygo Blaxell <ce3g8jdj@umail.furryterror.org>,
	Dmitriy Gorokh <Dmitriy_Gorokh@epam.com>
Subject: RE: [RFC][V8][PATCH 0/5] btrfs: allocation_hint mode
Date: Tue, 14 Dec 2021 01:03:37 +0000	[thread overview]
Message-ID: <SN6PR08MB44004972FE9C2794BA8FE48ED4759@SN6PR08MB4400.namprd08.prod.outlook.com> (raw)
In-Reply-To: <Ybe34gfrxl8O89PZ@localhost.localdomain>

Panasas would be very happy with this patch - it solves a problem we have when delete-only workloads thrash the disks. Because it is equivalent to "rm -rf" of an old directory, caching does't help since the metadata of the files being deleted will have fallen out of bcache long ago. So giving up a small amount of SSD space to always hold the metadata yields a big performance improvement.

We got as far as verifying that it doesn't break existing xfstests, we are open to writing the test cases you listed and also to check that the device out-of-space cases behave as expected.

Thanks,
Shafeeq

-----Original Message-----
From: Josef Bacik <josef@toxicpanda.com> 
Sent: Monday, December 13, 2021 4:15 PM
To: kreijack@inwind.it
Cc: David Sterba <dsterba@suse.cz>; Sinnamohideen, Shafeeq <shafeeqs@panasas.com>; Paul Jones <paul@pauljones.id.au>; linux-btrfs@vger.kernel.org; Zygo Blaxell <ce3g8jdj@umail.furryterror.org>
Subject: Re: [RFC][V8][PATCH 0/5] btrfs: allocation_hint mode

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.

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,

Josef