diff mbox series

[6/6] btrfs: use the filemap_fdatawrite_wbc helper for delalloc shrinking

Message ID 2acb56dd851d31d7b5547099821f0cbf6dfb5d29.1624894102.git.josef@toxicpanda.com (mailing list archive)
State New
Headers show
Series ENOSPC delalloc flushing fixes | expand

Commit Message

Josef Bacik June 28, 2021, 3:37 p.m. UTC
sync_inode() has some holes that can cause problems if we're under heavy
ENOSPC pressure.  If there's writeback running on a separate thread
sync_inode() will skip writing the inode altogether.  What we really
want is to make sure writeback has been started on all the pages to make
sure we can see the ordered extents and wait on them if appropriate.
Switch to this new helper which will allow us to accomplish this and
avoid ENOSPC'ing early.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Christoph Hellwig June 28, 2021, 4:50 p.m. UTC | #1
On Mon, Jun 28, 2021 at 11:37:11AM -0400, Josef Bacik wrote:
> sync_inode() has some holes that can cause problems if we're under heavy
> ENOSPC pressure.  If there's writeback running on a separate thread
> sync_inode() will skip writing the inode altogether.  What we really
> want is to make sure writeback has been started on all the pages to make
> sure we can see the ordered extents and wait on them if appropriate.
> Switch to this new helper which will allow us to accomplish this and
> avoid ENOSPC'ing early.

The only other exported user of sync_inode is in btrfs as well.  What
is the difference vs this caller?  Mostly I'd like to kill sync_inode
to reduce the surface of different hooks into the writeback code, and
for something externally callable your new filemap_fdatawrite_wbc
helpers looks nassively preferable of sync_inode /
writeback_single_inode.
Josef Bacik June 28, 2021, 6:13 p.m. UTC | #2
On 6/28/21 12:50 PM, Christoph Hellwig wrote:
> On Mon, Jun 28, 2021 at 11:37:11AM -0400, Josef Bacik wrote:
>> sync_inode() has some holes that can cause problems if we're under heavy
>> ENOSPC pressure.  If there's writeback running on a separate thread
>> sync_inode() will skip writing the inode altogether.  What we really
>> want is to make sure writeback has been started on all the pages to make
>> sure we can see the ordered extents and wait on them if appropriate.
>> Switch to this new helper which will allow us to accomplish this and
>> avoid ENOSPC'ing early.
> 
> The only other exported user of sync_inode is in btrfs as well.  What
> is the difference vs this caller?  Mostly I'd like to kill sync_inode
> to reduce the surface of different hooks into the writeback code, and
> for something externally callable your new filemap_fdatawrite_wbc
> helpers looks nassively preferable of sync_inode /
> writeback_single_inode.
> 

Sounds good, if you don't hate this helper I'll go through and clean up all the 
various helpers.  Thanks,

Josef
diff mbox series

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e388153c4ae4..b25c84aba743 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9713,7 +9713,7 @@  static int start_delalloc_inodes(struct btrfs_root *root,
 			btrfs_queue_work(root->fs_info->flush_workers,
 					 &work->work);
 		} else {
-			ret = sync_inode(inode, wbc);
+			ret = filemap_fdatawrite_wbc(inode->i_mapping, wbc);
 			btrfs_add_delayed_iput(inode);
 			if (ret || wbc->nr_to_write <= 0)
 				goto out;