From: Greg KH <greg@kroah.com>
To: Nikolay Borisov <nborisov@suse.com>
Cc: stable@vger.kernel.org, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: Ensure replaced device doesn't have pending chunk allocation
Date: Fri, 5 Jul 2019 13:01:36 +0200 [thread overview]
Message-ID: <20190705110136.GA14533@kroah.com> (raw)
In-Reply-To: <20190703094552.15833-2-nborisov@suse.com>
On Wed, Jul 03, 2019 at 12:45:49PM +0300, Nikolay Borisov wrote:
> Recent FITRIM work, namely bbbf7243d62d ("btrfs: combine device update
> operations during transaction commit") combined the way certain
> operations are recoded in a transaction. As a result an ASSERT was added
> in dev_replace_finish to ensure the new code works correctly.
> Unfortunately I got reports that it's possible to trigger the assert,
> meaning that during a device replace it's possible to have an unfinished
> chunk allocation on the source device.
>
> This is supposed to be prevented by the fact that a transaction is
> committed before finishing the replace oepration and alter acquiring the
> chunk mutex. This is not sufficient since by the time the transaction is
> committed and the chunk mutex acquired it's possible to allocate a chunk
> depending on the workload being executed on the replaced device. This
> bug has been present ever since device replace was introduced but there
> was never code which checks for it.
>
> The correct way to fix is to ensure that there is no pending device
> modification operation when the chunk mutex is acquire and if there is
> repeat transaction commit. Unfortunately it's not possible to just
> exclude the source device from btrfs_fs_devices::dev_alloc_list since
> this causes ENOSPC to be hit in transaction commit.
>
> Fixing that in another way would need to add special cases to handle the
> last writes and forbid new ones. The looped transaction fix is more
> obvious, and can be easily backported. The runtime of dev-replace is
> long so there's no noticeable delay caused by that.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>
> Hello Greg,
>
> Please merge the following backport of upstream commit debd1c065d2037919a7da67baf55cc683fee09f0
> to 4.4.y stable branch.
Thanks for all of these, now queued up.
greg k-h
next prev parent reply other threads:[~2019-07-05 11:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-03 9:45 [PATCH] btrfs: Ensure replaced device doesn't have pending chunk allocation Nikolay Borisov
2019-07-05 11:01 ` Greg KH [this message]
-- strict thread matches above, loose matches on Subject: below --
2019-07-03 9:45 Nikolay Borisov
2019-07-03 9:45 Nikolay Borisov
2019-07-03 9:45 Nikolay Borisov
2019-07-03 9:45 Nikolay Borisov
Reply instructions:
You may reply publicly 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=20190705110136.GA14533@kroah.com \
--to=greg@kroah.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=nborisov@suse.com \
--cc=stable@vger.kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).