| Message ID | 20230724132701.816771-2-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] btrfs: don't stop integrity writeback too early | expand |
On Mon, Jul 24, 2023 at 06:26:53AM -0700, Christoph Hellwig wrote: > extent_write_cache_pages stops writing pages as soon as nr_to_write hits > zero. That is the right thing for opportunistic writeback, but incorrect > for data integrity writeback, which needs to ensure that no dirty pages > are left in the range. Thus only stop the writeback for WB_SYNC_NONE > if nr_to_write hits 0. > > This is a port of write_cache_pages changes in commit 05fe478dd04e > ("mm: write_cache_pages integrity fix"). This makes sense to me. What is the reason the same reasoning doesn't apply to btree_write_cache_pages? Does the issue only happen in practice with fsync that no one is doing on the btree inode? It feels, in theory, we could do a writepages with SYNC_ALL and hit the same issue with pages going dirty and stealing the nr_writes. > > Note that I've only trigger the problem with other changes to the btrfs > writeback code, but this condition seems worthwhile fixing anyway. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/extent_io.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index c0440a0988c9a8..231e620e6c497d 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2098,7 +2098,8 @@ static int extent_write_cache_pages(struct address_space *mapping, > * We have to make sure to honor the new nr_to_write > * at any time > */ > - nr_to_write_done = wbc->nr_to_write <= 0; > + nr_to_write_done = wbc->sync_mode == WB_SYNC_NONE && > + wbc->nr_to_write <= 0; > } > folio_batch_release(&fbatch); > cond_resched(); > -- > 2.39.2 >
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index c0440a0988c9a8..231e620e6c497d 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -2098,7 +2098,8 @@ static int extent_write_cache_pages(struct address_space *mapping, * We have to make sure to honor the new nr_to_write * at any time */ - nr_to_write_done = wbc->nr_to_write <= 0; + nr_to_write_done = wbc->sync_mode == WB_SYNC_NONE && + wbc->nr_to_write <= 0; } folio_batch_release(&fbatch); cond_resched();
extent_write_cache_pages stops writing pages as soon as nr_to_write hits zero. That is the right thing for opportunistic writeback, but incorrect for data integrity writeback, which needs to ensure that no dirty pages are left in the range. Thus only stop the writeback for WB_SYNC_NONE if nr_to_write hits 0. This is a port of write_cache_pages changes in commit 05fe478dd04e ("mm: write_cache_pages integrity fix"). Note that I've only trigger the problem with other changes to the btrfs writeback code, but this condition seems worthwhile fixing anyway. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/extent_io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)