Linux-BTRFS Archive on lore.kernel.org
 help / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Nikolay Borisov <nborisov@suse.com>
Cc: Josef Bacik <josef@toxicpanda.com>,
	linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 4/8] btrfs: add ALLOC_CHUNK_FORCE to the flushing code
Date: Tue, 11 Dec 2018 17:47:51 +0100
Message-ID: <20181211164751.GN23615@twin.jikos.cz> (raw)
In-Reply-To: <83a7b193-0763-6f7a-611c-deaf0d6c380e@suse.com>

On Tue, Dec 11, 2018 at 12:08:23PM +0200, Nikolay Borisov wrote:
> 
> 
> On 3.12.18 г. 17:24 ч., Josef Bacik wrote:
> > With my change to no longer take into account the global reserve for
> > metadata allocation chunks we have this side-effect for mixed block
> > group fs'es where we are no longer allocating enough chunks for the
> > data/metadata requirements.  To deal with this add a ALLOC_CHUNK_FORCE
> > step to the flushing state machine.  This will only get used if we've
> > already made a full loop through the flushing machinery and tried
> > committing the transaction.  If we have then we can try and force a
> > chunk allocation since we likely need it to make progress.  This
> > resolves the issues I was seeing with the mixed bg tests in xfstests
> > with my previous patch.
> > 
> > Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> 
> Imo this and the previous patch should be squashed into one.

I don't see why, separate patches also look good to me. One changes the
logic regarding global reserve and the other fixes behaviour regarding
mixed block groups.

Possibly, if the fix can be applied first and then the overall logic
changed, that's still 2 patches but there's no intermediate state with
the bug. As long as it's not something really disasterous or if the
"one logical thing per patch" is unnecessarily split to 2 patches, I'm
willing to take more patches. This is a bit of a grey zone so if I'm
missing something regarding the split, please let me know.

  reply index

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-03 15:24 [PATCH 0/8][V2] Enospc cleanups and fixeS Josef Bacik
2018-12-03 15:24 ` [PATCH 1/8] btrfs: check if free bgs for commit Josef Bacik
2018-12-03 15:24 ` [PATCH 2/8] btrfs: dump block_rsv whe dumping space info Josef Bacik
2018-12-03 15:24 ` [PATCH 3/8] btrfs: don't use global rsv for chunk allocation Josef Bacik
2018-12-11  9:59   ` Nikolay Borisov
2018-12-03 15:24 ` [PATCH 4/8] btrfs: add ALLOC_CHUNK_FORCE to the flushing code Josef Bacik
2018-12-11 10:08   ` Nikolay Borisov
2018-12-11 16:47     ` David Sterba [this message]
2018-12-11 16:51       ` Nikolay Borisov
2018-12-11 19:04         ` David Sterba
2018-12-03 15:24 ` [PATCH 5/8] btrfs: don't enospc all tickets on flush failure Josef Bacik
2018-12-11 14:32   ` Nikolay Borisov
2018-12-03 15:24 ` [PATCH 6/8] btrfs: loop in inode_rsv_refill Josef Bacik
2018-12-12 16:01   ` Nikolay Borisov
2019-02-06 18:20     ` David Sterba
2019-01-30 16:41   ` David Sterba
2018-12-03 15:24 ` [PATCH 7/8] btrfs: be more explicit about allowed flush states Josef Bacik
2018-12-11 18:28   ` David Sterba
2018-12-12  8:40   ` Nikolay Borisov
2018-12-03 15:24 ` [PATCH 8/8] btrfs: reserve extra space during evict() Josef Bacik
2018-12-14  8:20   ` Nikolay Borisov
2018-12-13 14:11 ` [PATCH 0/8][V2] Enospc cleanups and fixeS David Sterba
2018-12-13 14:36   ` Nikolay Borisov
2018-12-13 14:45   ` Josef Bacik
2018-12-13 18:17     ` David Sterba
2018-12-13 18:28       ` Josef Bacik
2018-12-13 18:41         ` David Sterba
2019-02-08 16:08 ` David Sterba
  -- strict thread matches above, loose matches on Subject: below --
2018-11-21 19:03 [PATCH 0/8] Enospc cleanups and fixes Josef Bacik
2018-11-21 19:03 ` [PATCH 4/8] btrfs: add ALLOC_CHUNK_FORCE to the flushing code Josef Bacik
2018-11-26 11:28   ` Nikolay Borisov

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181211164751.GN23615@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=nborisov@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox