public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Cc: Hans van Kranenburg <hans@knorrie.org>
Subject: [PATCH RFC 2/2] btrfs: space-info: Don't allow signal to interrupt ticket waiting
Date: Mon,  6 Jul 2020 15:44:35 +0800	[thread overview]
Message-ID: <20200706074435.52356-3-wqu@suse.com> (raw)
In-Reply-To: <20200706074435.52356-1-wqu@suse.com>

[BUG]
When balance receive a fatal signal, it can make the fs to read-only
mode if the timing is unlucky enough:

  BTRFS info (device xvdb): balance: start -d -m -s
  BTRFS info (device xvdb): relocating block group 73001861120 flags metadata
  BTRFS info (device xvdb): found 12236 extents, stage: move data extents
  BTRFS info (device xvdb): relocating block group 71928119296 flags data
  BTRFS info (device xvdb): found 3 extents, stage: move data extents
  BTRFS info (device xvdb): found 3 extents, stage: update data pointers
  BTRFS info (device xvdb): relocating block group 60922265600 flags metadata
  BTRFS: error (device xvdb) in btrfs_drop_snapshot:5505: errno=-4 unknown
  BTRFS info (device xvdb): forced readonly
  BTRFS info (device xvdb): balance: ended with status: -4

[CAUSE]
This is caused by the fact that btrfs ticketing space system can be
interrupted, and cause all kind of -EINTR returned to various critical
section, where we never thought of -EINTR at all.

Even for things like btrfs_start_transaction() can be affected by
signal:
 btrfs_start_transaction()
 |- start_transaction(flush = FLUSH_ALL)
    |- btrfs_block_rsv_add()
       |- btrfs_reserve_metadata_bytes()
          |- __reserve_metadata_bytes()
             |- handle_reserve_ticket()
                |- wait_reserve_ticket()
                   |- prepare_to_wait_event(TASK_KILLABLE)
                   |- ticket->error = -EINTR;

And all related callers get -EINTR error.

In fact, there are really very limited call sites can really handle that
-EINTR properly, above btrfs_drop_snapshot() is one case.

[FIX]
Things like metadata allocation is really a critical section for btrfs,
we don't really want it to be that killable by some impatient users.

In fact, for really long duration calls, it should have their own checks
on signal, like balance, reflink, generic fiemap calls.

So this patch will make ticket waiting uninterruptible, relying on each
long duration calls to handle their signals more properly.

Reported-by: Hans van Kranenburg <hans@knorrie.org>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/space-info.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index c7bd3fdd7792..c5cfc759b804 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1099,7 +1099,8 @@ static void wait_reserve_ticket(struct btrfs_fs_info *fs_info,
 
 	spin_lock(&space_info->lock);
 	while (ticket->bytes > 0 && ticket->error == 0) {
-		ret = prepare_to_wait_event(&ticket->wait, &wait, TASK_KILLABLE);
+		ret = prepare_to_wait_event(&ticket->wait, &wait,
+					    TASK_UNINTERRUPTIBLE);
 		if (ret) {
 			/*
 			 * Delete us from the list. After we unlock the space
-- 
2.27.0


  parent reply	other threads:[~2020-07-06  7:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06  7:44 [PATCH RFC 0/2] btrfs: make ticket wait uninterruptible to address unexpected RO during balance Qu Wenruo
2020-07-06  7:44 ` [PATCH RFC 1/2] btrfs: relocation: Allow signal to cancel balance Qu Wenruo
2020-07-06 13:45   ` Josef Bacik
2020-07-06 18:19   ` Hans van Kranenburg
2020-07-06 22:43     ` Qu Wenruo
2020-07-06  7:44 ` Qu Wenruo [this message]
2020-07-06 13:45   ` [PATCH RFC 2/2] btrfs: space-info: Don't allow signal to interrupt ticket waiting Josef Bacik
2020-07-06 13:50     ` Qu Wenruo
2020-07-06 13:53       ` Josef Bacik
2020-07-06 14:05         ` Qu Wenruo
2020-07-06 14:33           ` Josef Bacik
2020-07-07 16:16             ` David Sterba

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=20200706074435.52356-3-wqu@suse.com \
    --to=wqu@suse.com \
    --cc=hans@knorrie.org \
    --cc=linux-btrfs@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