Re: [PATCH 0/3] Balance vs device add fixes

[Josef Bacik <josef@toxicpanda.com>]

On Mon, Nov 01, 2021 at 01:53:21PM +0200, Nikolay Borisov wrote:
> This series enables adding of a device when balance is paused (i.e an fs is mounted
> with skip_balance options). This is needed to give users a chance to gracefully
> handle an ENOSPC situation in the face of running balance. To achieve this introduce
> a new exclop - BALANCE_PAUSED which is made compatible with device add. More
> details in each patche.
> 
> I've tested this with an fstests which I will be posting in a bit.
> 
> Nikolay Borisov (3):
>   btrfs: introduce BTRFS_EXCLOP_BALANCE_PAUSED exclusive state
>   btrfs: make device add compatible with paused balance in
>     btrfs_exclop_start_try_lock
>   btrfs: allow device add if balance is paused
> 
>  fs/btrfs/ctree.h   |  1 +
>  fs/btrfs/ioctl.c   | 49 +++++++++++++++++++++++++++++++++++++++-------
>  fs/btrfs/volumes.c | 23 ++++++++++++++++++----
>  fs/btrfs/volumes.h |  2 +-
>  4 files changed, 63 insertions(+), 12 deletions(-)
> 

A few things

1) Can we integrate the flipping into helpers?  Something like

	btrfs_exclop_change_state(PAUSED);

   So the locking and stuff is all with the code that messes with the exclop?

2) The existing helpers do WRITE_ONCE(), is that needed here?  I assume not
   because we're not actually exiting our exclop state, but still seems wonky.

3) Maybe have an __btrfs_exclop_finish(type), so instead of 

	if (paused) {
		do thing;
	} else {
		btrfs_exclop_finish();
	}

  you can instead do

	type = BTRFS_EXCLOP_NONE;
	if (pause stuff) {
		do things;
		type = BTRFS_EXCLOP_BALANCE_PAUSED;
	}

	/* other stuff. */
	__btrfs_exclop_finish(type);

then btrfs_exclop_finish just does __btrfs_exclop_finish(NONE);

Thanks,

Josef