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

On Mon, Dec 13, 2021 at 05:49:22PM -0500, Zygo Blaxell wrote:

On Mon, Dec 13, 2021 at 04:15:14PM -0500, Josef Bacik wrote:

quoted

On Mon, Dec 13, 2021 at 08:54:24PM +0100, Goffredo Baroncelli wrote:

quoted

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.

I've been running earlier versions with modifications since summer 2020,
and this version mostly unmodified (rebase changes only) since it was
posted.  It seems to work, even in corner cases like converting balances,
replacing drives, and running out of space.  The "running out of space"
experience is on btrfs is weird at the best of times, and these patches
add some more new special cases, but it doesn't behave in ways that
would surprise a sysadmin familiar with how btrfs chunk allocation works.

One major piece that's missing is adjusting the statvfs (aka df)
available blocks field so that it doesn't include unallocated space on
any metadata-only devices.  Right now all the unallocated space on
metadata-only devices is counted as free even though it's impossible to
put a data block there, so anything that is triggered automatically
on "f_bavail < some_threshold" will be confused.

I don't think that piece has to block the rest of the patch series--if
you're not using the feature, df gives the right number (or at least the
same number it gave before), and if you are using the feature, you can
subtract the unavailable data space until a later patch comes along to
fix it.

I like

	echo data_only > /sys/fs/btrfs/$uuid/devinfo/3/type

more than patching btrfs-progs so I can use

	btrfs prop set /dev/... allocation_hint data_only

but I admit that might be because I'm weird.

Ok this is good enough for me.  Refresh the series Goffredo and I'll review it,
get some xfstests as I described, and fix up btrfs-progs to show the preferences
via either btrfs filesystem show or some other mechanism since the df thing will
be confusing.  We'll assume if you're using this feature you are aware of the
space gotchas and then fix up bugs as they show up.  With the testing and user
feedback tho this looks good enough for me, I just want to make sure we have
tests and such in place before merging.  Thanks,

Josef