* [PATCH v3 0/3] protect relocation with sb_start_write
@ 2022-03-28 6:29 Naohiro Aota
2022-03-28 6:29 ` [PATCH v3 1/3] btrfs: mark resumed async balance as writing Naohiro Aota
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Naohiro Aota @ 2022-03-28 6:29 UTC (permalink / raw)
To: linux-btrfs; +Cc: johannes.thumshirn, Naohiro Aota
This series is a follow-up to the series below. The old series added
an assertion to btrfs_relocate_chunk() to check if it is protected
with sb_start_write(). However, it revealed another location we need
to add sb_start_write() [1].
https://lore.kernel.org/linux-btrfs/cover.1645157220.git.naohiro.aota@wdc.com/T/
[1] https://lore.kernel.org/linux-btrfs/cover.1645157220.git.naohiro.aota@wdc.com/T/#e06eecc07ce1cd1e45bfd30a374bd2d15b4fd76d8
Patch 1 adds sb_{start,end}_write() to the resumed async balancing.
Patches 2 and 3 add an ASSERT to catch a future error.
--
v3:
- Only add sb_write_started(), which we really use. (Dave Chinner)
- Drop patch "btrfs: mark device addition as mnt_want_write_file" (Filipe Manana)
- Narrow asserting region to btrfs_relocate_block_group() and check only
when the BG is data BG. (Filipe Manana)
v2:
- Use mnt_want_write_file() instead of sb_start_write() for
btrfs_ioctl_add_dev()
- Drop bogus fixes tag
- Change the stable target to meaningful 4.9+
Naohiro Aota (3):
btrfs: mark resumed async balance as writing
fs: add check functions for sb_start_{write,pagefault,intwrite}
btrfs: assert that relocation is protected with sb_start_write()
fs/btrfs/relocation.c | 4 ++++
fs/btrfs/volumes.c | 2 ++
include/linux/fs.h | 5 +++++
3 files changed, 11 insertions(+)
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] btrfs: mark resumed async balance as writing
2022-03-28 6:29 [PATCH v3 0/3] protect relocation with sb_start_write Naohiro Aota
@ 2022-03-28 6:29 ` Naohiro Aota
2022-03-28 6:29 ` [PATCH v3 2/3] fs: add check functions for sb_start_{write,pagefault,intwrite} Naohiro Aota
2022-03-28 6:29 ` [PATCH v3 3/3] btrfs: assert that relocation is protected with sb_start_write() Naohiro Aota
2 siblings, 0 replies; 6+ messages in thread
From: Naohiro Aota @ 2022-03-28 6:29 UTC (permalink / raw)
To: linux-btrfs; +Cc: johannes.thumshirn, Naohiro Aota, Filipe Manana
When btrfs balance is interrupted with umount, the background balance
resumes on the next mount. There is a potential deadlock with FS freezing
here like as described in commit 26559780b953 ("btrfs: zoned: mark
relocation as writing"). Mark the process as sb_writing to avoid it.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Cc: stable@vger.kernel.org # 4.9+
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
fs/btrfs/volumes.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 3fd17e87815a..3471698fd831 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4430,10 +4430,12 @@ static int balance_kthread(void *data)
struct btrfs_fs_info *fs_info = data;
int ret = 0;
+ sb_start_write(fs_info->sb);
mutex_lock(&fs_info->balance_mutex);
if (fs_info->balance_ctl)
ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);
mutex_unlock(&fs_info->balance_mutex);
+ sb_end_write(fs_info->sb);
return ret;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] fs: add check functions for sb_start_{write,pagefault,intwrite}
2022-03-28 6:29 [PATCH v3 0/3] protect relocation with sb_start_write Naohiro Aota
2022-03-28 6:29 ` [PATCH v3 1/3] btrfs: mark resumed async balance as writing Naohiro Aota
@ 2022-03-28 6:29 ` Naohiro Aota
2022-03-28 10:40 ` Filipe Manana
2022-03-28 6:29 ` [PATCH v3 3/3] btrfs: assert that relocation is protected with sb_start_write() Naohiro Aota
2 siblings, 1 reply; 6+ messages in thread
From: Naohiro Aota @ 2022-03-28 6:29 UTC (permalink / raw)
To: linux-btrfs; +Cc: johannes.thumshirn, Naohiro Aota, Filipe Manana
Add a function sb_write_started() to return if sb_start_write() is
properly called. It is used in the next commit.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
include/linux/fs.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 27746a3da8fd..57fedc4af4a1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1732,6 +1732,11 @@ static inline bool __sb_start_write_trylock(struct super_block *sb, int level)
#define __sb_writers_release(sb, lev) \
percpu_rwsem_release(&(sb)->s_writers.rw_sem[(lev)-1], 1, _THIS_IP_)
+static inline bool sb_write_started(struct super_block *sb)
+{
+ return lockdep_is_held_type(sb->s_writers.rw_sem + SB_FREEZE_WRITE - 1, 1);
+}
+
/**
* sb_end_write - drop write access to a superblock
* @sb: the super we wrote to
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] btrfs: assert that relocation is protected with sb_start_write()
2022-03-28 6:29 [PATCH v3 0/3] protect relocation with sb_start_write Naohiro Aota
2022-03-28 6:29 ` [PATCH v3 1/3] btrfs: mark resumed async balance as writing Naohiro Aota
2022-03-28 6:29 ` [PATCH v3 2/3] fs: add check functions for sb_start_{write,pagefault,intwrite} Naohiro Aota
@ 2022-03-28 6:29 ` Naohiro Aota
2022-03-28 10:49 ` Filipe Manana
2 siblings, 1 reply; 6+ messages in thread
From: Naohiro Aota @ 2022-03-28 6:29 UTC (permalink / raw)
To: linux-btrfs; +Cc: johannes.thumshirn, Naohiro Aota
btrfs_relocate_chunk() initiates new ordered extents. They can cause a
hang when a process is trying to thaw the filesystem.
We should have called sb_start_write(), so the filesystem is not being
frozen. Add an ASSERT to check it is protected.
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
fs/btrfs/relocation.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index fdc2c4b411f0..705799b2b207 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3977,6 +3977,10 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
if (!bg)
return -ENOENT;
+ /* Assert we called sb_start_write(), not to race with FS freezing */
+ if (bg->flags & BTRFS_BLOCK_GROUP_DATA)
+ ASSERT(sb_write_started(fs_info->sb));
+
if (btrfs_pinned_by_swapfile(fs_info, bg)) {
btrfs_put_block_group(bg);
return -ETXTBSY;
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/3] fs: add check functions for sb_start_{write,pagefault,intwrite}
2022-03-28 6:29 ` [PATCH v3 2/3] fs: add check functions for sb_start_{write,pagefault,intwrite} Naohiro Aota
@ 2022-03-28 10:40 ` Filipe Manana
0 siblings, 0 replies; 6+ messages in thread
From: Filipe Manana @ 2022-03-28 10:40 UTC (permalink / raw)
To: Naohiro Aota; +Cc: linux-btrfs, johannes.thumshirn, Filipe Manana
On Mon, Mar 28, 2022 at 03:29:21PM +0900, Naohiro Aota wrote:
> Add a function sb_write_started() to return if sb_start_write() is
> properly called. It is used in the next commit.
>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
> include/linux/fs.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 27746a3da8fd..57fedc4af4a1 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1732,6 +1732,11 @@ static inline bool __sb_start_write_trylock(struct super_block *sb, int level)
> #define __sb_writers_release(sb, lev) \
> percpu_rwsem_release(&(sb)->s_writers.rw_sem[(lev)-1], 1, _THIS_IP_)
>
> +static inline bool sb_write_started(struct super_block *sb)
The argument can be made const.
Also, the subject "fs: add check functions for sb_start_{write,pagefault,intwrite}" is
now oudated, since only sb_write_started() is added.
Thanks.
> +{
> + return lockdep_is_held_type(sb->s_writers.rw_sem + SB_FREEZE_WRITE - 1, 1);
> +}
> +
> /**
> * sb_end_write - drop write access to a superblock
> * @sb: the super we wrote to
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 3/3] btrfs: assert that relocation is protected with sb_start_write()
2022-03-28 6:29 ` [PATCH v3 3/3] btrfs: assert that relocation is protected with sb_start_write() Naohiro Aota
@ 2022-03-28 10:49 ` Filipe Manana
0 siblings, 0 replies; 6+ messages in thread
From: Filipe Manana @ 2022-03-28 10:49 UTC (permalink / raw)
To: Naohiro Aota; +Cc: linux-btrfs, johannes.thumshirn
On Mon, Mar 28, 2022 at 03:29:22PM +0900, Naohiro Aota wrote:
> btrfs_relocate_chunk() initiates new ordered extents.
Just say the relocation of data block groups creates ordered extents.
Saying btrfs_relocate_chunk() is a bit misleading, since the ordered
extents are created way deeper in the call chain.
> They can cause a
> hang when a process is trying to thaw the filesystem.
>
> We should have called sb_start_write(), so the filesystem is not being
> frozen. Add an ASSERT to check it is protected.
>
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
> fs/btrfs/relocation.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index fdc2c4b411f0..705799b2b207 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -3977,6 +3977,10 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
> if (!bg)
> return -ENOENT;
>
> + /* Assert we called sb_start_write(), not to race with FS freezing */
Sentences should end with punctuation.
The comment is also a bit vague.
Just say that relocation of data block groups creates ordered extents, and
without sb_start_write() protection, we can deadlock if a fs freeze is started
before the relocation completes, because finishing an ordered extent requires
joining a transaction.
Otherwise it looks good, thanks.
> + if (bg->flags & BTRFS_BLOCK_GROUP_DATA)
> + ASSERT(sb_write_started(fs_info->sb));
> +
> if (btrfs_pinned_by_swapfile(fs_info, bg)) {
> btrfs_put_block_group(bg);
> return -ETXTBSY;
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-28 10:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-28 6:29 [PATCH v3 0/3] protect relocation with sb_start_write Naohiro Aota
2022-03-28 6:29 ` [PATCH v3 1/3] btrfs: mark resumed async balance as writing Naohiro Aota
2022-03-28 6:29 ` [PATCH v3 2/3] fs: add check functions for sb_start_{write,pagefault,intwrite} Naohiro Aota
2022-03-28 10:40 ` Filipe Manana
2022-03-28 6:29 ` [PATCH v3 3/3] btrfs: assert that relocation is protected with sb_start_write() Naohiro Aota
2022-03-28 10:49 ` Filipe Manana
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.