linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] writeback: inode cgroup wb switch should not call ihold()
@ 2016-06-15 20:35 Tahsin Erdogan
  2016-06-16  7:13 ` Jan Kara
  0 siblings, 1 reply; 3+ messages in thread
From: Tahsin Erdogan @ 2016-06-15 20:35 UTC (permalink / raw)
  To: Jens Axboe, Tejun Heo, Alexander Viro, Jan Kara
  Cc: linux-fsdevel, linux-kernel, Tahsin Erdogan

Asynchronous wb switching of inodes takes an additional ref count on an
inode to make sure inode remains valid until switchover is completed.

However, anyone calling ihold() must already have a ref count on inode,
but in this case inode->i_count may already be zero:

------------[ cut here ]------------
WARNING: CPU: 1 PID: 917 at fs/inode.c:397 ihold+0x2b/0x30
CPU: 1 PID: 917 Comm: kworker/u4:5 Not tainted 4.7.0-rc2+ #49
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs
01/01/2011
Workqueue: writeback wb_workfn (flush-8:16)
 0000000000000000 ffff88007ca0fb58 ffffffff805990af 0000000000000000
 0000000000000000 ffff88007ca0fb98 ffffffff80268702 0000018d000004e2
 ffff88007cef40e8 ffff88007c9b89a8 ffff880079e3a740 0000000000000003
Call Trace:
 [<ffffffff805990af>] dump_stack+0x4d/0x6e
 [<ffffffff80268702>] __warn+0xc2/0xe0
 [<ffffffff802687d8>] warn_slowpath_null+0x18/0x20
 [<ffffffff8035b4ab>] ihold+0x2b/0x30
 [<ffffffff80367ecc>] inode_switch_wbs+0x11c/0x180
 [<ffffffff80369110>] wbc_detach_inode+0x170/0x1a0
 [<ffffffff80369abc>] writeback_sb_inodes+0x21c/0x530
 [<ffffffff80369f7e>] wb_writeback+0xee/0x1e0
 [<ffffffff8036a147>] wb_workfn+0xd7/0x280
 [<ffffffff80287531>] ? try_to_wake_up+0x1b1/0x2b0
 [<ffffffff8027bb09>] process_one_work+0x129/0x300
 [<ffffffff8027be06>] worker_thread+0x126/0x480
 [<ffffffff8098cde7>] ? __schedule+0x1c7/0x561
 [<ffffffff8027bce0>] ? process_one_work+0x300/0x300
 [<ffffffff80280ff4>] kthread+0xc4/0xe0
 [<ffffffff80335578>] ? kfree+0xc8/0x100
 [<ffffffff809903cf>] ret_from_fork+0x1f/0x40
 [<ffffffff80280f30>] ? __kthread_parkme+0x70/0x70
---[ end trace aaefd2fd9f306bc4 ]---

Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Tahsin Erdogan <tahsin@google.com>
---
v2:
 removed inode->i_count == 0 check
 replaced ihold() with __iget()
 updated commit description

 fs/fs-writeback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 989a2cef6b76..fe7e83a45eff 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -483,9 +483,9 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
 		goto out_free;
 	}
 	inode->i_state |= I_WB_SWITCH;
+	__iget(inode);
 	spin_unlock(&inode->i_lock);
 
-	ihold(inode);
 	isw->inode = inode;
 
 	atomic_inc(&isw_nr_in_flight);
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] writeback: inode cgroup wb switch should not call ihold()
  2016-06-15 20:35 [PATCH v2] writeback: inode cgroup wb switch should not call ihold() Tahsin Erdogan
@ 2016-06-16  7:13 ` Jan Kara
  2016-06-16 12:09   ` Tahsin Erdogan
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2016-06-16  7:13 UTC (permalink / raw)
  To: Tahsin Erdogan
  Cc: Jens Axboe, Tejun Heo, Alexander Viro, Jan Kara, linux-fsdevel,
	linux-kernel

On Wed 15-06-16 13:35:10, Tahsin Erdogan wrote:
> Asynchronous wb switching of inodes takes an additional ref count on an
> inode to make sure inode remains valid until switchover is completed.
> 
> However, anyone calling ihold() must already have a ref count on inode,
> but in this case inode->i_count may already be zero:
> 
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 917 at fs/inode.c:397 ihold+0x2b/0x30
> CPU: 1 PID: 917 Comm: kworker/u4:5 Not tainted 4.7.0-rc2+ #49
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs
> 01/01/2011
> Workqueue: writeback wb_workfn (flush-8:16)
>  0000000000000000 ffff88007ca0fb58 ffffffff805990af 0000000000000000
>  0000000000000000 ffff88007ca0fb98 ffffffff80268702 0000018d000004e2
>  ffff88007cef40e8 ffff88007c9b89a8 ffff880079e3a740 0000000000000003
> Call Trace:
>  [<ffffffff805990af>] dump_stack+0x4d/0x6e
>  [<ffffffff80268702>] __warn+0xc2/0xe0
>  [<ffffffff802687d8>] warn_slowpath_null+0x18/0x20
>  [<ffffffff8035b4ab>] ihold+0x2b/0x30
>  [<ffffffff80367ecc>] inode_switch_wbs+0x11c/0x180
>  [<ffffffff80369110>] wbc_detach_inode+0x170/0x1a0
>  [<ffffffff80369abc>] writeback_sb_inodes+0x21c/0x530
>  [<ffffffff80369f7e>] wb_writeback+0xee/0x1e0
>  [<ffffffff8036a147>] wb_workfn+0xd7/0x280
>  [<ffffffff80287531>] ? try_to_wake_up+0x1b1/0x2b0
>  [<ffffffff8027bb09>] process_one_work+0x129/0x300
>  [<ffffffff8027be06>] worker_thread+0x126/0x480
>  [<ffffffff8098cde7>] ? __schedule+0x1c7/0x561
>  [<ffffffff8027bce0>] ? process_one_work+0x300/0x300
>  [<ffffffff80280ff4>] kthread+0xc4/0xe0
>  [<ffffffff80335578>] ? kfree+0xc8/0x100
>  [<ffffffff809903cf>] ret_from_fork+0x1f/0x40
>  [<ffffffff80280f30>] ? __kthread_parkme+0x70/0x70
> ---[ end trace aaefd2fd9f306bc4 ]---
> 
> Acked-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Tahsin Erdogan <tahsin@google.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] writeback: inode cgroup wb switch should not call ihold()
  2016-06-16  7:13 ` Jan Kara
@ 2016-06-16 12:09   ` Tahsin Erdogan
  0 siblings, 0 replies; 3+ messages in thread
From: Tahsin Erdogan @ 2016-06-16 12:09 UTC (permalink / raw)
  To: Jan Kara
  Cc: Jens Axboe, Tejun Heo, Alexander Viro, Jan Kara, linux-fsdevel,
	linux-kernel

On Thu, Jun 16, 2016 at 12:13 AM, Jan Kara <jack@suse.cz> wrote:
> Looks good. You can add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks Jan and Tejun!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-06-16 12:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-15 20:35 [PATCH v2] writeback: inode cgroup wb switch should not call ihold() Tahsin Erdogan
2016-06-16  7:13 ` Jan Kara
2016-06-16 12:09   ` Tahsin Erdogan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).