| Message ID | bb12e8c269c6dd67496aa868cef2a7c4bc75d292.1668530684.git.rgoldwyn@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Lock extents before pages | expand |
On Tue, Nov 15, 2022 at 12:00:27PM -0600, Goldwyn Rodrigues wrote: > writepages() writebacks the unwritten pages the synchronously. So we > know that once writepages returns, the pages are "done" and can be > safely unlocked. However, with async writes, this is done over a thread. > So, for async writeback, perform this within the async thread. > > Locking is performed at origin of async_chunk and unlocked when > async_chunk completes. > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > --- > fs/btrfs/inode.c | 33 +++++++++++++++++++++++++++------ > 1 file changed, 27 insertions(+), 6 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 92726831dd5d..aa393219019b 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -528,6 +528,7 @@ struct async_chunk { > struct list_head extents; > struct cgroup_subsys_state *blkcg_css; > struct btrfs_work work; > + struct extent_state *cached_state; I'd rather this be separated into it's own patch, I definitely got confused for a second. > struct async_cow *async_cow; > }; > > @@ -1491,6 +1492,9 @@ static noinline void async_cow_start(struct btrfs_work *work) > > compressed_extents = compress_file_range(async_chunk); > if (compressed_extents == 0) { > + unlock_extent(&async_chunk->inode->io_tree, > + async_chunk->start, async_chunk->end, > + &async_chunk->cached_state); > btrfs_add_delayed_iput(async_chunk->inode); > async_chunk->inode = NULL; > } > @@ -1530,11 +1534,16 @@ static noinline void async_cow_free(struct btrfs_work *work) > struct async_cow *async_cow; > > async_chunk = container_of(work, struct async_chunk, work); > - if (async_chunk->inode) > + if (async_chunk->inode) { > + unlock_extent(&async_chunk->inode->io_tree, > + async_chunk->start, async_chunk->end, > + &async_chunk->cached_state); > btrfs_add_delayed_iput(async_chunk->inode); > + } > if (async_chunk->blkcg_css) > css_put(async_chunk->blkcg_css); > > + Extra whitespace. > async_cow = async_chunk->async_cow; > if (atomic_dec_and_test(&async_cow->num_chunks)) > kvfree(async_cow); > @@ -1558,7 +1567,6 @@ static int cow_file_range_async(struct btrfs_inode *inode, > unsigned nofs_flag; > const blk_opf_t write_flags = wbc_to_write_flags(wbc); > > - unlock_extent(&inode->io_tree, start, end, NULL); > > if (inode->flags & BTRFS_INODE_NOCOMPRESS && > !btrfs_test_opt(fs_info, FORCE_COMPRESS)) { > @@ -1600,6 +1608,9 @@ static int cow_file_range_async(struct btrfs_inode *inode, > ihold(&inode->vfs_inode); > async_chunk[i].async_cow = ctx; > async_chunk[i].inode = inode; > + async_chunk[i].cached_state = NULL; > + btrfs_lock_and_flush_ordered_range(inode, start, cur_end, > + &async_chunk[i].cached_state); > async_chunk[i].start = start; > async_chunk[i].end = cur_end; > async_chunk[i].write_flags = write_flags; > @@ -8222,10 +8233,11 @@ static int btrfs_writepages(struct address_space *mapping, > struct writeback_control *wbc) > { > u64 start, end; > - struct inode *inode = mapping->host; > + struct btrfs_inode *inode = BTRFS_I(mapping->host); > struct extent_state *cached = NULL; > + bool async_wb; > int ret; > - u64 isize = round_up(i_size_read(inode), PAGE_SIZE) - 1; > + u64 isize = round_up(i_size_read(&inode->vfs_inode), PAGE_SIZE) - 1; > > if (wbc->range_cyclic) { > start = mapping->writeback_index << PAGE_SHIFT; > @@ -8239,9 +8251,18 @@ static int btrfs_writepages(struct address_space *mapping, > if (start >= end) > return 0; > > - lock_extent(&BTRFS_I(inode)->io_tree, start, end, &cached); > + /* > + * For async I/O, locking and unlocking is performed with the > + * allocation and completion of async_chunk. > + */ > + async_wb = btrfs_inode_can_compress(inode) && > + inode_need_compress(inode, start, end); These can change their answer arbitrarily and randomly, which means we could end up doing an async extent when we didn't think we would, and then unpleasantness happens. Thanks, Josef
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 92726831dd5d..aa393219019b 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -528,6 +528,7 @@ struct async_chunk { struct list_head extents; struct cgroup_subsys_state *blkcg_css; struct btrfs_work work; + struct extent_state *cached_state; struct async_cow *async_cow; }; @@ -1491,6 +1492,9 @@ static noinline void async_cow_start(struct btrfs_work *work) compressed_extents = compress_file_range(async_chunk); if (compressed_extents == 0) { + unlock_extent(&async_chunk->inode->io_tree, + async_chunk->start, async_chunk->end, + &async_chunk->cached_state); btrfs_add_delayed_iput(async_chunk->inode); async_chunk->inode = NULL; } @@ -1530,11 +1534,16 @@ static noinline void async_cow_free(struct btrfs_work *work) struct async_cow *async_cow; async_chunk = container_of(work, struct async_chunk, work); - if (async_chunk->inode) + if (async_chunk->inode) { + unlock_extent(&async_chunk->inode->io_tree, + async_chunk->start, async_chunk->end, + &async_chunk->cached_state); btrfs_add_delayed_iput(async_chunk->inode); + } if (async_chunk->blkcg_css) css_put(async_chunk->blkcg_css); + async_cow = async_chunk->async_cow; if (atomic_dec_and_test(&async_cow->num_chunks)) kvfree(async_cow); @@ -1558,7 +1567,6 @@ static int cow_file_range_async(struct btrfs_inode *inode, unsigned nofs_flag; const blk_opf_t write_flags = wbc_to_write_flags(wbc); - unlock_extent(&inode->io_tree, start, end, NULL); if (inode->flags & BTRFS_INODE_NOCOMPRESS && !btrfs_test_opt(fs_info, FORCE_COMPRESS)) { @@ -1600,6 +1608,9 @@ static int cow_file_range_async(struct btrfs_inode *inode, ihold(&inode->vfs_inode); async_chunk[i].async_cow = ctx; async_chunk[i].inode = inode; + async_chunk[i].cached_state = NULL; + btrfs_lock_and_flush_ordered_range(inode, start, cur_end, + &async_chunk[i].cached_state); async_chunk[i].start = start; async_chunk[i].end = cur_end; async_chunk[i].write_flags = write_flags; @@ -8222,10 +8233,11 @@ static int btrfs_writepages(struct address_space *mapping, struct writeback_control *wbc) { u64 start, end; - struct inode *inode = mapping->host; + struct btrfs_inode *inode = BTRFS_I(mapping->host); struct extent_state *cached = NULL; + bool async_wb; int ret; - u64 isize = round_up(i_size_read(inode), PAGE_SIZE) - 1; + u64 isize = round_up(i_size_read(&inode->vfs_inode), PAGE_SIZE) - 1; if (wbc->range_cyclic) { start = mapping->writeback_index << PAGE_SHIFT; @@ -8239,9 +8251,18 @@ static int btrfs_writepages(struct address_space *mapping, if (start >= end) return 0; - lock_extent(&BTRFS_I(inode)->io_tree, start, end, &cached); + /* + * For async I/O, locking and unlocking is performed with the + * allocation and completion of async_chunk. + */ + async_wb = btrfs_inode_can_compress(inode) && + inode_need_compress(inode, start, end); + if (!async_wb) + lock_extent(&inode->io_tree, start, end, &cached); ret = extent_writepages(mapping, wbc); - unlock_extent(&BTRFS_I(inode)->io_tree, start, end, &cached); + if (!async_wb) + unlock_extent(&inode->io_tree, start, end, &cached); + return ret; }
writepages() writebacks the unwritten pages the synchronously. So we know that once writepages returns, the pages are "done" and can be safely unlocked. However, with async writes, this is done over a thread. So, for async writeback, perform this within the async thread. Locking is performed at origin of async_chunk and unlocked when async_chunk completes. Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> --- fs/btrfs/inode.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-)