| Message ID | 3c5403c65f3aacbba5f4e441b336233f2112f141.1648448228.git.naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | protect relocation with sb_start_write | expand |
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 >
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;
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(+)