From: David Sterba <dsterba@suse.cz>
To: fdmanana@kernel.org
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] Btrfs: avoid deadlock with memory reclaim due to allocation of devices
Date: Wed, 9 Jan 2019 19:26:53 +0100 [thread overview]
Message-ID: <20190109182653.GN23615@twin.jikos.cz> (raw)
In-Reply-To: <20181213211725.14832-1-fdmanana@kernel.org>
On Thu, Dec 13, 2018 at 09:17:25PM +0000, fdmanana@kernel.org wrote:
> - if (list_empty(&fs_devices->resized_devices))
> - return;
> -
> - mutex_lock(&fs_devices->device_list_mutex);
> mutex_lock(&fs_info->chunk_mutex);
> list_for_each_entry_safe(curr, next, &fs_devices->resized_devices,
> resized_list) {
> @@ -7309,7 +7306,6 @@ void btrfs_update_commit_device_size(struct btrfs_fs_info *fs_info)
> curr->commit_total_bytes = curr->disk_total_bytes;
I'm not sure about removing the device_list_mutex that's said to protect
the commit_total_bytes (comment in struct btrfs_device).
Otherwise the logic is ok, the double lock could happen as you describe.
btrfs_update_commit_device_size is called from btrfs_commit_transaction,
at the same time as commit_bytes_used. The latter is handled in a
similar way in btrfs_update_commit_device_bytes_used, but does not take
the device_list_mutex.
commit_total_bytes is checked several times (eg. in write_dev_supers) to
see if writing the superblock copy is still within the device range.
So, without the protected change, it's theoretically possible that a
stale value is used for the test and the superblock is either written
though it should not, and the other way around.
This would require a resize racing at the time of the check. Grow and
shrink seem to take chunk_mutex while adjusting all the total/size
values, but it's not actually easy to follow as sometimes there are
helpers like btrfs_device_set_total_bytes used and sometimes it's direct
access.
That the device_list_mutex can be safely dropped probably follows from
the simple fact that btrfs_update_commit_device_bytes_used is called
before write_dev_supers in the same context.
But this sounds too simple, given that there are locks taken and
released and btrfs_write_and_wait_transaction called between.
Referencing this code:
2201 btrfs_update_commit_device_size(fs_info);
2202 btrfs_update_commit_device_bytes_used(cur_trans);
2203
2204 clear_bit(BTRFS_FS_LOG1_ERR, &fs_info->flags);
2205 clear_bit(BTRFS_FS_LOG2_ERR, &fs_info->flags);
2206
2207 btrfs_trans_release_chunk_metadata(trans);
2208
2209 spin_lock(&fs_info->trans_lock);
2210 cur_trans->state = TRANS_STATE_UNBLOCKED;
2211 fs_info->running_transaction = NULL;
2212 spin_unlock(&fs_info->trans_lock);
2213 mutex_unlock(&fs_info->reloc_mutex);
2214
2215 wake_up(&fs_info->transaction_wait);
2216
2217 ret = btrfs_write_and_wait_transaction(trans);
2218 if (ret) {
2219 btrfs_handle_fs_error(fs_info, ret,
2220 "Error while writing out transaction");
2221 mutex_unlock(&fs_info->tree_log_mutex);
2222 goto scrub_continue;
2223 }
2224
2225 ret = write_all_supers(fs_info, 0);
next prev parent reply other threads:[~2019-01-09 18:27 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-13 21:17 [PATCH] Btrfs: avoid deadlock with memory reclaim due to allocation of devices fdmanana
2018-12-14 7:27 ` Nikolay Borisov
2019-01-08 11:51 ` Filipe Manana
2019-01-09 18:26 ` David Sterba [this message]
2019-01-09 19:48 ` Filipe Manana
2019-01-10 7:32 ` Anand Jain
2019-01-10 7:03 ` Nikolay Borisov
2019-01-11 17:17 ` [PATCH v2] " fdmanana
2019-01-14 8:21 ` Anand Jain
2019-01-18 18:07 ` David Sterba
2019-01-25 2:56 ` Anand Jain
2019-01-25 3:40 ` Anand Jain
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=20190109182653.GN23615@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=fdmanana@kernel.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 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.