diff mbox series

[20/21] btrfs: Add inode->i_count instead of calling ihold()

Message ID c201e92c0a69df45a8f1c4651028ccfb30aebdd2.1677793433.git.rgoldwyn@suse.com (mailing list archive)
State New, archived
Headers show
Series Lock extents before pages | expand

Commit Message

Goldwyn Rodrigues March 2, 2023, 10:25 p.m. UTC
From: Goldwyn Rodrigues <rgoldwyn@suse.com>

I am not sure of this patch, but put it to avoid the WARN_ON() in
ihold().  I am not sure why the i_count would drop below one at this
point of time since this is still called within writepages context.

Perhaps, there is a better way to solve this?
---
 fs/btrfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Christoph Hellwig March 7, 2023, 5:06 p.m. UTC | #1
On Thu, Mar 02, 2023 at 04:25:05PM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> 
> I am not sure of this patch, but put it to avoid the WARN_ON() in
> ihold().  I am not sure why the i_count would drop below one at this
> point of time since this is still called within writepages context.
> 
> Perhaps, there is a better way to solve this?

How do you trigger the warning?  Basically i_count could only be
0 when doing writeback from inode evication, and just incrementing
i_count blindly will do the wrong thing there.
Goldwyn Rodrigues March 8, 2023, 11:03 p.m. UTC | #2
On  9:06 07/03, Christoph Hellwig wrote:
> On Thu, Mar 02, 2023 at 04:25:05PM -0600, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > 
> > I am not sure of this patch, but put it to avoid the WARN_ON() in
> > ihold().  I am not sure why the i_count would drop below one at this
> > point of time since this is still called within writepages context.
> > 
> > Perhaps, there is a better way to solve this?
> 
> How do you trigger the warning?  Basically i_count could only be
> 0 when doing writeback from inode evication, and just incrementing
> i_count blindly will do the wrong thing there.

Without this patch, performing a writeback with async writeback
(mount option compress) will trigger this warning.

Yes, this patch is incorrect. However we have to hold on to an inode
reference in order to complete the asynchronous writeback.
Christoph Hellwig March 9, 2023, 9:14 a.m. UTC | #3
On Wed, Mar 08, 2023 at 05:03:57PM -0600, Goldwyn Rodrigues wrote:
> Without this patch, performing a writeback with async writeback
> (mount option compress) will trigger this warning.

What is the trace in the warning?
Goldwyn Rodrigues March 11, 2023, 3:52 a.m. UTC | #4
On  1:14 09/03, Christoph Hellwig wrote:
> On Wed, Mar 08, 2023 at 05:03:57PM -0600, Goldwyn Rodrigues wrote:
> > Without this patch, performing a writeback with async writeback
> > (mount option compress) will trigger this warning.
> 
> What is the trace in the warning?
[   57.105512] ------------[ cut here ]------------
[   57.108857] WARNING: CPU: 3 PID: 1631 at fs/inode.c:451 ihold+0x23/0x30
[   57.111887] Modules linked in:
[   57.113984] CPU: 3 PID: 1631 Comm: kworker/u8:9 Not tainted 6.3.0-rc1-dave+ #22 a352fb29779d7031315b84505284616e0ef1983c
[   57.117994] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.0-0-gd239552-rebuilt.opensuse.org 04/01/2014
[   57.122344] Workqueue: writeback wb_workfn (flush-btrfs-5)
[   57.125073] RIP: 0010:ihold+0x23/0x30
[   57.127342] Code: 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 b8 01 00 00 00 f0 0f c1 87 08 02 00 00 83 c0 01 83 f8 01 7e 05 c3 cc cc cc cc <0f> 0b c3 cc cc cc cc 66 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90
[   57.134365] RSP: 0018:ffffb0f28128faa0 EFLAGS: 00010246
[   57.137069] RAX: 0000000000000001 RBX: ffff9cae8a320e18 RCX: 0000000000000000
[   57.140224] RDX: ffff9cae8a320e18 RSI: ffff9cae849c5300 RDI: ffff9cae96d430a8
[   57.143447] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
[   57.146644] R10: ffff9cae849c5308 R11: ffff9cae97640e38 R12: 000000000019a000
[   57.149808] R13: 000000000019afff R14: ffffffffa948e340 R15: 0000000000000000
[   57.153383] FS:  0000000000000000(0000) GS:ffff9caefbd80000(0000) knlGS:0000000000000000
[   57.157007] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   57.160028] CR2: 0000563ecc4d0230 CR3: 000000010aafa006 CR4: 0000000000370ee0
[   57.163259] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   57.166647] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   57.169588] Call Trace:
[   57.171486]  <TASK>
[   57.173625]  btrfs_writepages+0x3ea/0x820
[   57.175995]  do_writepages+0xd5/0x1a0
[   57.178061]  ? lock_is_held_type+0xad/0x120
[   57.180605]  __writeback_single_inode+0x54/0x630
[   57.182981]  writeback_sb_inodes+0x1fc/0x560
[   57.185249]  wb_writeback+0xc5/0x480
[   57.187273]  wb_workfn+0x84/0x650
[   57.189207]  ? lock_acquire+0xc8/0x310
[   57.191033]  ? process_one_work+0x23c/0x630
[   57.192998]  ? lock_is_held_type+0xad/0x120
[   57.194800]  process_one_work+0x2c0/0x630
[   57.196619]  worker_thread+0x50/0x3d0
[   57.198129]  ? __pfx_worker_thread+0x10/0x10
[   57.200005]  kthread+0xea/0x110
[   57.201397]  ? __pfx_kthread+0x10/0x10
[   57.202897]  ret_from_fork+0x2c/0x50
[   57.204490]  </TASK>
diff mbox series

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c4e5eb5d9ee4..b5f5c1896dbb 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1535,7 +1535,7 @@  static int cow_file_range_async(struct btrfs_inode *inode,
 		 * igrab is called higher up in the call chain, take only the
 		 * lightweight reference for the callback lifetime
 		 */
-		ihold(&inode->vfs_inode);
+		atomic_inc(&inode->vfs_inode.i_count);
 		async_chunk[i].async_cow = ctx;
 		async_chunk[i].inode = inode;
 		async_chunk[i].start = start;