All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.