linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
@ 2017-07-18 17:25 Christoph Hellwig
  2017-07-18 18:15 ` Darrick J. Wong
  2017-07-19 13:11 ` Nikolay Borisov
  0 siblings, 2 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-07-18 17:25 UTC (permalink / raw)
  To: n.borisov.lkml, linux-xfs

Just like in the allocator we must avoid touching multiple AGs out of
order when freeing blocks, as freeing still locks the AGF and can cause
the same AB-BA deadlocks as in the allocation path.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Nikolay Borisov <n.borisov.lkml@gmail.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 0a9880777c9c..935adde72a8b 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5435,6 +5435,7 @@ __xfs_bunmapi(
 	xfs_fsblock_t		sum;
 	xfs_filblks_t		len = *rlen;	/* length to unmap in file */
 	xfs_fileoff_t		max_len;
+	xfs_agnumber_t		prev_agno = NULLAGNUMBER, agno;
 
 	trace_xfs_bunmap(ip, bno, len, flags, _RET_IP_);
 
@@ -5534,6 +5535,17 @@ __xfs_bunmapi(
 		 */
 		del = got;
 		wasdel = isnullstartblock(del.br_startblock);
+
+		/*
+		 * Make sure we don't touch multiple AGF headers out of order
+		 * in a single transaction, as that could cause AB-BA deadlocks.
+		 */
+		if (!wasdel) {
+			agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
+			if (prev_agno != NULLAGNUMBER && prev_agno > agno)
+				break;
+			prev_agno = agno;
+		}
 		if (got.br_startoff < start) {
 			del.br_startoff = start;
 			del.br_blockcount -= start - got.br_startoff;
-- 
2.11.0


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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-18 17:25 [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi Christoph Hellwig
@ 2017-07-18 18:15 ` Darrick J. Wong
  2017-07-18 18:27   ` Nikolay Borisov
  2017-07-19 13:11 ` Nikolay Borisov
  1 sibling, 1 reply; 17+ messages in thread
From: Darrick J. Wong @ 2017-07-18 18:15 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: n.borisov.lkml, linux-xfs

On Tue, Jul 18, 2017 at 07:25:45PM +0200, Christoph Hellwig wrote:
> Just like in the allocator we must avoid touching multiple AGs out of
> order when freeing blocks, as freeing still locks the AGF and can cause
> the same AB-BA deadlocks as in the allocation path.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Nikolay Borisov <n.borisov.lkml@gmail.com>

Looks ok, though I wonder what the bug report looked like. :)

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 0a9880777c9c..935adde72a8b 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5435,6 +5435,7 @@ __xfs_bunmapi(
>  	xfs_fsblock_t		sum;
>  	xfs_filblks_t		len = *rlen;	/* length to unmap in file */
>  	xfs_fileoff_t		max_len;
> +	xfs_agnumber_t		prev_agno = NULLAGNUMBER, agno;
>  
>  	trace_xfs_bunmap(ip, bno, len, flags, _RET_IP_);
>  
> @@ -5534,6 +5535,17 @@ __xfs_bunmapi(
>  		 */
>  		del = got;
>  		wasdel = isnullstartblock(del.br_startblock);
> +
> +		/*
> +		 * Make sure we don't touch multiple AGF headers out of order
> +		 * in a single transaction, as that could cause AB-BA deadlocks.
> +		 */
> +		if (!wasdel) {
> +			agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
> +			if (prev_agno != NULLAGNUMBER && prev_agno > agno)
> +				break;
> +			prev_agno = agno;
> +		}
>  		if (got.br_startoff < start) {
>  			del.br_startoff = start;
>  			del.br_blockcount -= start - got.br_startoff;
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-18 18:15 ` Darrick J. Wong
@ 2017-07-18 18:27   ` Nikolay Borisov
  2017-07-18 18:33     ` Darrick J. Wong
  0 siblings, 1 reply; 17+ messages in thread
From: Nikolay Borisov @ 2017-07-18 18:27 UTC (permalink / raw)
  To: Darrick J. Wong, Christoph Hellwig; +Cc: linux-xfs



On 18.07.2017 21:15, Darrick J. Wong wrote:
> On Tue, Jul 18, 2017 at 07:25:45PM +0200, Christoph Hellwig wrote:
>> Just like in the allocator we must avoid touching multiple AGs out of
>> order when freeing blocks, as freeing still locks the AGF and can cause
>> the same AB-BA deadlocks as in the allocation path.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Reported-by: Nikolay Borisov <n.borisov.lkml@gmail.com>
> 
> Looks ok, though I wonder what the bug report looked like. :)

https://www.spinics.net/lists/linux-xfs/msg05918.html


> 
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
>> ---
>>  fs/xfs/libxfs/xfs_bmap.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
>> index 0a9880777c9c..935adde72a8b 100644
>> --- a/fs/xfs/libxfs/xfs_bmap.c
>> +++ b/fs/xfs/libxfs/xfs_bmap.c
>> @@ -5435,6 +5435,7 @@ __xfs_bunmapi(
>>  	xfs_fsblock_t		sum;
>>  	xfs_filblks_t		len = *rlen;	/* length to unmap in file */
>>  	xfs_fileoff_t		max_len;
>> +	xfs_agnumber_t		prev_agno = NULLAGNUMBER, agno;
>>  
>>  	trace_xfs_bunmap(ip, bno, len, flags, _RET_IP_);
>>  
>> @@ -5534,6 +5535,17 @@ __xfs_bunmapi(
>>  		 */
>>  		del = got;
>>  		wasdel = isnullstartblock(del.br_startblock);
>> +
>> +		/*
>> +		 * Make sure we don't touch multiple AGF headers out of order
>> +		 * in a single transaction, as that could cause AB-BA deadlocks.
>> +		 */
>> +		if (!wasdel) {
>> +			agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
>> +			if (prev_agno != NULLAGNUMBER && prev_agno > agno)
>> +				break;
>> +			prev_agno = agno;
>> +		}
>>  		if (got.br_startoff < start) {
>>  			del.br_startoff = start;
>>  			del.br_blockcount -= start - got.br_startoff;
>> -- 
>> 2.11.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-18 18:27   ` Nikolay Borisov
@ 2017-07-18 18:33     ` Darrick J. Wong
  2017-07-19  7:36       ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Darrick J. Wong @ 2017-07-18 18:33 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Christoph Hellwig, linux-xfs

On Tue, Jul 18, 2017 at 09:27:43PM +0300, Nikolay Borisov wrote:
> 
> 
> On 18.07.2017 21:15, Darrick J. Wong wrote:
> > On Tue, Jul 18, 2017 at 07:25:45PM +0200, Christoph Hellwig wrote:
> >> Just like in the allocator we must avoid touching multiple AGs out of
> >> order when freeing blocks, as freeing still locks the AGF and can cause
> >> the same AB-BA deadlocks as in the allocation path.
> >>
> >> Signed-off-by: Christoph Hellwig <hch@lst.de>
> >> Reported-by: Nikolay Borisov <n.borisov.lkml@gmail.com>
> > 
> > Looks ok, though I wonder what the bug report looked like. :)
> 
> https://www.spinics.net/lists/linux-xfs/msg05918.html

Ah, ok.  That's what I suspected, so I'm glad to have confirmation.

--D

> 
> 
> > 
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> >> ---
> >>  fs/xfs/libxfs/xfs_bmap.c | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> >> index 0a9880777c9c..935adde72a8b 100644
> >> --- a/fs/xfs/libxfs/xfs_bmap.c
> >> +++ b/fs/xfs/libxfs/xfs_bmap.c
> >> @@ -5435,6 +5435,7 @@ __xfs_bunmapi(
> >>  	xfs_fsblock_t		sum;
> >>  	xfs_filblks_t		len = *rlen;	/* length to unmap in file */
> >>  	xfs_fileoff_t		max_len;
> >> +	xfs_agnumber_t		prev_agno = NULLAGNUMBER, agno;
> >>  
> >>  	trace_xfs_bunmap(ip, bno, len, flags, _RET_IP_);
> >>  
> >> @@ -5534,6 +5535,17 @@ __xfs_bunmapi(
> >>  		 */
> >>  		del = got;
> >>  		wasdel = isnullstartblock(del.br_startblock);
> >> +
> >> +		/*
> >> +		 * Make sure we don't touch multiple AGF headers out of order
> >> +		 * in a single transaction, as that could cause AB-BA deadlocks.
> >> +		 */
> >> +		if (!wasdel) {
> >> +			agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
> >> +			if (prev_agno != NULLAGNUMBER && prev_agno > agno)
> >> +				break;
> >> +			prev_agno = agno;
> >> +		}
> >>  		if (got.br_startoff < start) {
> >>  			del.br_startoff = start;
> >>  			del.br_blockcount -= start - got.br_startoff;
> >> -- 
> >> 2.11.0
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-18 18:33     ` Darrick J. Wong
@ 2017-07-19  7:36       ` Christoph Hellwig
  0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-07-19  7:36 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Nikolay Borisov, Christoph Hellwig, linux-xfs

On Tue, Jul 18, 2017 at 11:33:51AM -0700, Darrick J. Wong wrote:
> On Tue, Jul 18, 2017 at 09:27:43PM +0300, Nikolay Borisov wrote:
> > 
> > 
> > On 18.07.2017 21:15, Darrick J. Wong wrote:
> > > On Tue, Jul 18, 2017 at 07:25:45PM +0200, Christoph Hellwig wrote:
> > >> Just like in the allocator we must avoid touching multiple AGs out of
> > >> order when freeing blocks, as freeing still locks the AGF and can cause
> > >> the same AB-BA deadlocks as in the allocation path.
> > >>
> > >> Signed-off-by: Christoph Hellwig <hch@lst.de>
> > >> Reported-by: Nikolay Borisov <n.borisov.lkml@gmail.com>
> > > 
> > > Looks ok, though I wonder what the bug report looked like. :)
> > 
> > https://www.spinics.net/lists/linux-xfs/msg05918.html
> 
> Ah, ok.  That's what I suspected, so I'm glad to have confirmation.

Btw, I found a little bug when rebasing additional bunmapi work on
top of the patch - we need to add a !isrt to the conditional for
the AG check.  But I'll wait for test results from Nikolay before
resending it, as it should not affect normal non-RT setups.

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-18 17:25 [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi Christoph Hellwig
  2017-07-18 18:15 ` Darrick J. Wong
@ 2017-07-19 13:11 ` Nikolay Borisov
  2017-07-19 15:09   ` Nikolay Borisov
  1 sibling, 1 reply; 17+ messages in thread
From: Nikolay Borisov @ 2017-07-19 13:11 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs



On 18.07.2017 20:25, Christoph Hellwig wrote:
> Just like in the allocator we must avoid touching multiple AGs out of
> order when freeing blocks, as freeing still locks the AGF and can cause
> the same AB-BA deadlocks as in the allocation path.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.d> Reported-by: Nikolay Borisov <n.borisov.lkml@gmail.com>

Unfortunately I this patch is not enough. I just tested on a 3.12 kernel ( the issue was easiest to repro there) 
and I still get stack traces. I will also test on a 4.4-based kernel as well but I assume the results are going to
be the same. 

Excerpt: 

[ 8167.898643] SysRq : Show Blocked State
[ 8167.902006] fio             D 0000000000000005     0 15744  15690 0x00000000
[ 8167.902006]  ffff88007f81d390 0000000000000086 0000000000000002 ffff8800952720a0
[ 8167.902006]  ffff88007f81dfd8 ffff88007f81dfd8 ffff88007f81dfd8 ffff8800952720a0
[ 8167.902006]  ffff88007a462f30 7fffffffffffffff ffff8800952720a0 0000000000000001
[ 8167.902006] Call Trace:
[ 8167.902006]  [<ffffffff81524ff9>] schedule_timeout+0x1d9/0x2a0
[ 8167.902006]  [<ffffffff81528960>] __down+0x70/0x100
[ 8167.902006]  [<ffffffff810a231c>] down+0x3c/0x50
[ 8167.902006]  [<ffffffffa01e0b13>] xfs_buf_lock+0x33/0x100 [xfs]
[ 8167.902006]  [<ffffffffa01e0dce>] _xfs_buf_find+0x1ee/0x420 [xfs]
[ 8167.902006]  [<ffffffffa01e12d3>] xfs_buf_get_map+0x23/0x200 [xfs]
[ 8167.902006]  [<ffffffffa01e2141>] xfs_buf_read_map+0x21/0x160 [xfs]
[ 8167.902006]  [<ffffffffa0254423>] xfs_trans_read_buf_map+0x413/0x5d0 [xfs]
[ 8167.902006]  [<ffffffffa0200561>] xfs_read_agf+0xa1/0x150 [xfs]
[ 8167.902006]  [<ffffffffa020066d>] xfs_alloc_read_agf+0x5d/0x180 [xfs]
[ 8167.902006]  [<ffffffffa0200c00>] xfs_alloc_fix_freelist+0x430/0x4c0 [xfs]
[ 8167.902006]  [<ffffffffa0200ebb>] xfs_alloc_vextent+0x22b/0x7b0 [xfs]
[ 8167.902006]  [<ffffffffa02143f7>] xfs_bmap_btalloc+0x407/0x960 [xfs]
[ 8167.902006]  [<ffffffffa021506e>] __xfs_bmapi_allocate+0xde/0x340 [xfs]
[ 8167.902006]  [<ffffffffa01ddc55>] xfs_bmapi_allocate+0xa5/0xb0 [xfs]
[ 8167.902006]  [<ffffffffa0215914>] xfs_bmapi_write+0x644/0xab0 [xfs]
[ 8167.902006]  [<ffffffffa01ef589>] xfs_iomap_write_direct+0x1e9/0x390 [xfs]
[ 8167.902006]  [<ffffffffa01d9f24>] __xfs_get_blocks+0x494/0x930 [xfs]
[ 8167.902006]  [<ffffffff811efbdc>] do_blockdev_direct_IO+0xadc/0x2ea0
[ 8167.902006]  [<ffffffffa01d8b48>] xfs_vm_direct_IO+0x78/0xa0 [xfs]
[ 8167.902006]  [<ffffffffa01e7751>] xfs_file_dio_aio_write.isra.9+0x171/0x430 [xfs]
[ 8167.902006]  [<ffffffffa01e7d35>] xfs_file_aio_write+0x155/0x170 [xfs]
[ 8167.902006]  [<ffffffff81201847>] do_io_submit+0x797/0x830
[ 8167.902006]  [<ffffffff81533249>] system_call_fastpath+0x16/0x1b
[ 8167.902006]  [<00007f73754296f7>] 0x7f73754296f6
[ 8167.902006] fio             D ffff8801394b20a0     0 15746  15690 0x00000000
[ 8167.902006]  ffff8800741c5390 0000000000000086 0000000000000010 ffff8801394b20a0
[ 8167.902006]  ffff8800741c5fd8 ffff8800741c5fd8 ffff8800741c5fd8 ffff8801394b20a0
[ 8167.902006]  ffff88007a462030 7fffffffffffffff ffff8801394b20a0 00000000015f9001
[ 8167.902006] Call Trace:
[ 8167.902006]  [<ffffffff81524ff9>] schedule_timeout+0x1d9/0x2a0
[ 8167.902006]  [<ffffffff81528960>] __down+0x70/0x100
[ 8167.902006]  [<ffffffff810a231c>] down+0x3c/0x50
[ 8167.902006]  [<ffffffffa01e0b13>] xfs_buf_lock+0x33/0x100 [xfs]
[ 8167.902006]  [<ffffffffa01e0dce>] _xfs_buf_find+0x1ee/0x420 [xfs]
[ 8167.902006]  [<ffffffffa01e12d3>] xfs_buf_get_map+0x23/0x200 [xfs]
[ 8167.902006]  [<ffffffffa01e2141>] xfs_buf_read_map+0x21/0x160 [xfs]
[ 8167.902006]  [<ffffffffa0254423>] xfs_trans_read_buf_map+0x413/0x5d0 [xfs]
[ 8167.902006]  [<ffffffffa0200561>] xfs_read_agf+0xa1/0x150 [xfs]
[ 8167.902006]  [<ffffffffa020066d>] xfs_alloc_read_agf+0x5d/0x180 [xfs]
[ 8167.902006]  [<ffffffffa0200c00>] xfs_alloc_fix_freelist+0x430/0x4c0 [xfs]
[ 8167.902006]  [<ffffffffa0200ebb>] xfs_alloc_vextent+0x22b/0x7b0 [xfs]
[ 8167.902006]  [<ffffffffa02143f7>] xfs_bmap_btalloc+0x407/0x960 [xfs]
[ 8167.902006]  [<ffffffffa021506e>] __xfs_bmapi_allocate+0xde/0x340 [xfs]
[ 8167.902006]  [<ffffffffa01ddc55>] xfs_bmapi_allocate+0xa5/0xb0 [xfs]
[ 8167.902006]  [<ffffffffa0215914>] xfs_bmapi_write+0x644/0xab0 [xfs]
[ 8167.902006]  [<ffffffffa01ef589>] xfs_iomap_write_direct+0x1e9/0x390 [xfs]
[ 8167.902006]  [<ffffffffa01d9f24>] __xfs_get_blocks+0x494/0x930 [xfs]
[ 8167.902006]  [<ffffffff811efbdc>] do_blockdev_direct_IO+0xadc/0x2ea0
[ 8167.902006]  [<ffffffffa01d8b48>] xfs_vm_direct_IO+0x78/0xa0 [xfs]
[ 8167.902006]  [<ffffffffa01e7751>] xfs_file_dio_aio_write.isra.9+0x171/0x430 [xfs]
[ 8167.902006]  [<ffffffffa01e7d35>] xfs_file_aio_write+0x155/0x170 [xfs]
[ 8167.902006]  [<ffffffff81201847>] do_io_submit+0x797/0x830
[ 8167.902006]  [<ffffffff81533249>] system_call_fastpath+0x16/0x1b
[ 8167.902006]  [<00007f73754296f7>] 0x7f73754296f6
[ 8167.902006] fio             D ffffffff81625900     0 15747  15690 0x00000000
[ 8167.902006]  ffff88004be11390 0000000000000086 00000000000003ff ffff8800b9a94140
[ 8167.902006]  ffff88004be11fd8 ffff88004be11fd8 ffff88004be11fd8 ffff8800b9a94140
[ 8167.902006]  ffff88007a462f30 7fffffffffffffff ffff8800b9a94140 0000000000000001
[ 8167.902006] Call Trace:
[ 8167.902006]  [<ffffffff81524ff9>] schedule_timeout+0x1d9/0x2a0
[ 8167.902006]  [<ffffffff81528960>] __down+0x70/0x100
[ 8167.902006]  [<ffffffff810a231c>] down+0x3c/0x50
[ 8167.902006]  [<ffffffffa01e0b13>] xfs_buf_lock+0x33/0x100 [xfs]
[ 8167.902006]  [<ffffffffa01e0dce>] _xfs_buf_find+0x1ee/0x420 [xfs]
[ 8167.902006]  [<ffffffffa01e12d3>] xfs_buf_get_map+0x23/0x200 [xfs]
[ 8167.902006]  [<ffffffffa01e2141>] xfs_buf_read_map+0x21/0x160 [xfs]
[ 8167.902006]  [<ffffffffa0254423>] xfs_trans_read_buf_map+0x413/0x5d0 [xfs]
[ 8167.902006]  [<ffffffffa0200561>] xfs_read_agf+0xa1/0x150 [xfs]
[ 8167.902006]  [<ffffffffa020066d>] xfs_alloc_read_agf+0x5d/0x180 [xfs]
[ 8167.902006]  [<ffffffffa0200c00>] xfs_alloc_fix_freelist+0x430/0x4c0 [xfs]
[ 8167.902006]  [<ffffffffa0200ebb>] xfs_alloc_vextent+0x22b/0x7b0 [xfs]
[ 8167.902006]  [<ffffffffa02143f7>] xfs_bmap_btalloc+0x407/0x960 [xfs]
[ 8167.902006]  [<ffffffffa021506e>] __xfs_bmapi_allocate+0xde/0x340 [xfs]
[ 8167.902006]  [<ffffffffa01ddc55>] xfs_bmapi_allocate+0xa5/0xb0 [xfs]
[ 8167.902006]  [<ffffffffa0215914>] xfs_bmapi_write+0x644/0xab0 [xfs]
[ 8167.902006]  [<ffffffffa01ef589>] xfs_iomap_write_direct+0x1e9/0x390 [xfs]
[ 8167.902006]  [<ffffffffa01d9f24>] __xfs_get_blocks+0x494/0x930 [xfs]
[ 8167.902006]  [<ffffffff811efbdc>] do_blockdev_direct_IO+0xadc/0x2ea0
[ 8167.902006]  [<ffffffffa01d8b48>] xfs_vm_direct_IO+0x78/0xa0 [xfs]
[ 8167.902006]  [<ffffffffa01e7751>] xfs_file_dio_aio_write.isra.9+0x171/0x430 [xfs]
[ 8167.902006]  [<ffffffffa01e7d35>] xfs_file_aio_write+0x155/0x170 [xfs]
[ 8167.902006]  [<ffffffff81201847>] do_io_submit+0x797/0x830
[ 8167.902006]  [<ffffffff81533249>] system_call_fastpath+0x16/0x1b
[ 8167.902006]  [<00007f73754296f7>] 0x7f73754296f6
[ 8167.902006] fio             D ffffffff81625900     0 15748  15690 0x00000000
[ 8167.902006]  ffff8800b5c35390 0000000000000086 00000000000003ff ffff8800b9a90000
[ 8167.902006]  ffff8800b5c35fd8 ffff8800b5c35fd8 ffff8800b5c35fd8 ffff8800b9a90000
[ 8167.902006]  ffff88007a462f30 7fffffffffffffff ffff8800b9a90000 0000000000000001
[ 8167.902006] Call Trace:
[ 8167.902006]  [<ffffffff81524ff9>] schedule_timeout+0x1d9/0x2a0
[ 8167.902006]  [<ffffffff81528960>] __down+0x70/0x100
[ 8167.902006]  [<ffffffffa01e0dce>] _xfs_buf_find+0x1ee/0x420 [xfs]
[ 8167.902006]  [<ffffffffa01e12d3>] xfs_buf_get_map+0x23/0x200 [xfs]
[ 8167.902006]  [<ffffffffa01e2141>] xfs_buf_read_map+0x21/0x160 [xfs]
[ 8167.902006]  [<ffffffffa0254423>] xfs_trans_read_buf_map+0x413/0x5d0 [xfs]
[ 8167.902006]  [<ffffffffa0200561>] xfs_read_agf+0xa1/0x150 [xfs]
[ 8167.902006]  [<ffffffffa020066d>] xfs_alloc_read_agf+0x5d/0x180 [xfs]
[ 8167.902006]  [<ffffffffa0200c00>] xfs_alloc_fix_freelist+0x430/0x4c0 [xfs]
[ 8167.902006]  [<ffffffffa0200ebb>] xfs_alloc_vextent+0x22b/0x7b0 [xfs]
[ 8167.902006]  [<ffffffffa02142ed>] xfs_bmap_btalloc+0x2fd/0x960 [xfs]
[ 8167.902006]  [<ffffffffa021506e>] __xfs_bmapi_allocate+0xde/0x340 [xfs]
[ 8167.902006]  [<ffffffffa01ddc55>] xfs_bmapi_allocate+0xa5/0xb0 [xfs]
[ 8167.902006]  [<ffffffffa0215914>] xfs_bmapi_write+0x644/0xab0 [xfs]
[ 8167.902006]  [<ffffffffa01ef589>] xfs_iomap_write_direct+0x1e9/0x390 [xfs]
[ 8167.902006]  [<ffffffffa01d9f24>] __xfs_get_blocks+0x494/0x930 [xfs]
[ 8167.902006]  [<ffffffff811efbdc>] do_blockdev_direct_IO+0xadc/0x2ea0
[ 8167.902006]  [<ffffffffa01d8b48>] xfs_vm_direct_IO+0x78/0xa0 [xfs]
[ 8167.902006]  [<ffffffffa01e7751>] xfs_file_dio_aio_write.isra.9+0x171/0x430 [xfs]
[ 8167.902006]  [<ffffffffa01e7d35>] xfs_file_aio_write+0x155/0x170 [xfs]
[ 8167.902006]  [<ffffffff81201847>] do_io_submit+0x797/0x830
[ 8167.902006]  [<ffffffff81533249>] system_call_fastpath+0x16/0x1b
[ 8167.902006]  [<00007f73754296f7>] 0x7f73754296f6
[ 8167.902006] fio             D ffffffff81625900     0 15749  15690 0x00000000
[ 8167.902006]  ffff8800b5c0fc10 0000000000000082 0000000000000003 ffff8800b8800000
[ 8167.902006]  ffff8800b5c0ffd8 ffff8800b5c0ffd8 ffff8800b5c0ffd8 ffff8800b8800000
[ 8167.902006]  ffff8800b5c0fd20 ffff8800b5c0fd18 7fffffffffffffff ffff8800b8800000
[ 8167.902006] xfs_io          D ffffffff81625900     0 15854  15342 0x00000000
[ 8167.902006]  ffff880095179910 0000000000000082 ffff8801396f9c80 ffff88004bc40000
[ 8167.902006]  ffff880095179fd8 ffff880095179fd8 ffff880095179fd8 ffff88004bc40000
[ 8167.902006]  ffff88007a462030 7fffffffffffffff ffff88004bc40000 00000000015f9001
[ 8167.902006] Call Trace:
[ 8167.902006]  [<ffffffff81524ff9>] schedule_timeout+0x1d9/0x2a0
[ 8167.902006]  [<ffffffff81528960>] __down+0x70/0x100
[ 8167.902006]  [<ffffffff810a231c>] down+0x3c/0x50
[ 8167.902006]  [<ffffffffa01e0b13>] xfs_buf_lock+0x33/0x100 [xfs]
[ 8167.902006]  [<ffffffffa01e0dce>] _xfs_buf_find+0x1ee/0x420 [xfs]
[ 8167.902006]  [<ffffffffa01e12d3>] xfs_buf_get_map+0x23/0x200 [xfs]
[ 8167.902006]  [<ffffffffa01e2141>] xfs_buf_read_map+0x21/0x160 [xfs]
[ 8167.902006]  [<ffffffffa0254423>] xfs_trans_read_buf_map+0x413/0x5d0 [xfs]
[ 8167.902006]  [<ffffffffa0200561>] xfs_read_agf+0xa1/0x150 [xfs]
[ 8167.902006]  [<ffffffffa020066d>] xfs_alloc_read_agf+0x5d/0x180 [xfs]
[ 8167.902006]  [<ffffffffa0200c00>] xfs_alloc_fix_freelist+0x430/0x4c0 [xfs]
[ 8167.902006]  [<ffffffffa0201509>] xfs_free_extent+0xc9/0x140 [xfs]
[ 8167.902006]  [<ffffffffa01dd69a>] xfs_bmap_finish+0x15a/0x1b0 [xfs]
[ 8167.902006]  [<ffffffffa023a21e>] xfs_itruncate_extents+0x2ce/0x470 [xfs]
[ 8167.902006]  [<ffffffffa01f19c3>] xfs_setattr_size+0x353/0x3f0 [xfs]
[ 8167.902006]  [<ffffffffa01f1b70>] xfs_vn_setattr+0x60/0x80 [xfs]
[ 8167.902006]  [<ffffffff811d0a51>] notify_change+0x241/0x3b0
[ 8167.902006]  [<ffffffff811b1b92>] do_truncate+0x62/0x90
[ 8167.902006]  [<ffffffff811b1f0b>] do_sys_ftruncate.constprop.11+0x12b/0x180
[ 8167.902006]  [<ffffffff81533249>] system_call_fastpath+0x16/0x1b
[ 8167.902006]  [<00007f5cb95239f7>] 0x7f5cb95239f6


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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-19 13:11 ` Nikolay Borisov
@ 2017-07-19 15:09   ` Nikolay Borisov
  2017-07-20  7:47     ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Nikolay Borisov @ 2017-07-19 15:09 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs



On 19.07.2017 16:11, Nikolay Borisov wrote:
> 
> 
> On 18.07.2017 20:25, Christoph Hellwig wrote:
>> Just like in the allocator we must avoid touching multiple AGs out of
>> order when freeing blocks, as freeing still locks the AGF and can cause
>> the same AB-BA deadlocks as in the allocation path.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.d> Reported-by: Nikolay Borisov <n.borisov.lkml@gmail.com>
> 
> Unfortunately I this patch is not enough. I just tested on a 3.12 kernel ( the issue was easiest to repro there) 
> and I still get stack traces. I will also test on a 4.4-based kernel as well but I assume the results are going to
> be the same. 

I can confirm that on 4.4 I also hit the same deadlock. So this patch is
not sufficient.

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-19 15:09   ` Nikolay Borisov
@ 2017-07-20  7:47     ` Christoph Hellwig
  2017-07-20  7:47       ` Nikolay Borisov
  0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2017-07-20  7:47 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Christoph Hellwig, linux-xfs

On Wed, Jul 19, 2017 at 06:09:29PM +0300, Nikolay Borisov wrote:
> I can confirm that on 4.4 I also hit the same deadlock. So this patch is
> not sufficient.

Odd.  And the patch to always just process a single extent from
xfs_itruncate_extents fixes the issue for you, right?

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-20  7:47     ` Christoph Hellwig
@ 2017-07-20  7:47       ` Nikolay Borisov
  2017-07-20  7:49         ` Christoph Hellwig
  0 siblings, 1 reply; 17+ messages in thread
From: Nikolay Borisov @ 2017-07-20  7:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs



On 20.07.2017 10:47, Christoph Hellwig wrote:
> Odd.  And the patch to always just process a single extent from
> xfs_itruncate_extents fixes the issue for you, right?

As far as I remember yes.

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-20  7:47       ` Nikolay Borisov
@ 2017-07-20  7:49         ` Christoph Hellwig
  2017-07-20  7:51           ` Nikolay Borisov
  2017-07-20 14:58           ` Nikolay Borisov
  0 siblings, 2 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-07-20  7:49 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Christoph Hellwig, linux-xfs

On Thu, Jul 20, 2017 at 10:47:50AM +0300, Nikolay Borisov wrote:
> 
> 
> On 20.07.2017 10:47, Christoph Hellwig wrote:
> > Odd.  And the patch to always just process a single extent from
> > xfs_itruncate_extents fixes the issue for you, right?
> 
> As far as I remember yes.

Can you retest it under otherwise identical conditions?

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-20  7:49         ` Christoph Hellwig
@ 2017-07-20  7:51           ` Nikolay Borisov
  2017-07-20 14:58           ` Nikolay Borisov
  1 sibling, 0 replies; 17+ messages in thread
From: Nikolay Borisov @ 2017-07-20  7:51 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs



On 20.07.2017 10:49, Christoph Hellwig wrote:
>>> Odd.  And the patch to always just process a single extent from
>>> xfs_itruncate_extents fixes the issue for you, right?
>> As far as I remember yes.
> Can you retest it under otherwise identical conditions?
Will report back when I have the results.

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-20  7:49         ` Christoph Hellwig
  2017-07-20  7:51           ` Nikolay Borisov
@ 2017-07-20 14:58           ` Nikolay Borisov
  2017-07-21 10:26             ` Christoph Hellwig
  2017-07-26 13:04             ` Christoph Hellwig
  1 sibling, 2 replies; 17+ messages in thread
From: Nikolay Borisov @ 2017-07-20 14:58 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs



On 20.07.2017 10:49, Christoph Hellwig wrote:
> On Thu, Jul 20, 2017 at 10:47:50AM +0300, Nikolay Borisov wrote:
>>
>>
>> On 20.07.2017 10:47, Christoph Hellwig wrote:
>>> Odd.  And the patch to always just process a single extent from
>>> xfs_itruncate_extents fixes the issue for you, right?
>>
>> As far as I remember yes.
> 
> Can you retest it under otherwise identical conditions?

Okay, I confirm that indeed changing the define from 2 to 1 fixes the
issue.

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-20 14:58           ` Nikolay Borisov
@ 2017-07-21 10:26             ` Christoph Hellwig
  2017-07-26 13:04             ` Christoph Hellwig
  1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-07-21 10:26 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Christoph Hellwig, linux-xfs

On Thu, Jul 20, 2017 at 05:58:42PM +0300, Nikolay Borisov wrote:
> Okay, I confirm that indeed changing the define from 2 to 1 fixes the
> issue.

Thanks for the report, although that leaves me puzzling why..

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-20 14:58           ` Nikolay Borisov
  2017-07-21 10:26             ` Christoph Hellwig
@ 2017-07-26 13:04             ` Christoph Hellwig
  2017-07-26 13:59               ` Nikolay Borisov
  2017-07-28  6:05               ` Nikolay Borisov
  1 sibling, 2 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-07-26 13:04 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Christoph Hellwig, linux-xfs

On Thu, Jul 20, 2017 at 05:58:42PM +0300, Nikolay Borisov wrote:
> 
> 
> On 20.07.2017 10:49, Christoph Hellwig wrote:
> > On Thu, Jul 20, 2017 at 10:47:50AM +0300, Nikolay Borisov wrote:
> >>
> >>
> >> On 20.07.2017 10:47, Christoph Hellwig wrote:
> >>> Odd.  And the patch to always just process a single extent from
> >>> xfs_itruncate_extents fixes the issue for you, right?
> >>
> >> As far as I remember yes.
> > 
> > Can you retest it under otherwise identical conditions?
> 
> Okay, I confirm that indeed changing the define from 2 to 1 fixes the
> issue.

But this is just on 4.4 and older, right?  We had issues in the
allocation path there as well, which should be fixed in 4.11 and later,
and in recrnt 4.9-stable releases.  Without those the allocation path
might sometimes lock out of order as well.

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-26 13:04             ` Christoph Hellwig
@ 2017-07-26 13:59               ` Nikolay Borisov
  2017-07-28  6:05               ` Nikolay Borisov
  1 sibling, 0 replies; 17+ messages in thread
From: Nikolay Borisov @ 2017-07-26 13:59 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs



On 26.07.2017 16:04, Christoph Hellwig wrote:
> But this is just on 4.4 and older, right?  We had issues in the
> allocation path there as well, which should be fixed in 4.11 and later,
> and in recrnt 4.9-stable releases.  Without those the allocation path
> might sometimes lock out of order as well.

I will try to repro on 4.11 WITHOUT your patch just to see that the
issue happens there as well. But if memory serves me right on newer
kernels it was much harder to hit (but not impossible).

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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-26 13:04             ` Christoph Hellwig
  2017-07-26 13:59               ` Nikolay Borisov
@ 2017-07-28  6:05               ` Nikolay Borisov
  2017-07-31 12:09                 ` Christoph Hellwig
  1 sibling, 1 reply; 17+ messages in thread
From: Nikolay Borisov @ 2017-07-28  6:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs



On 26.07.2017 16:04, Christoph Hellwig wrote:
> On Thu, Jul 20, 2017 at 05:58:42PM +0300, Nikolay Borisov wrote:
>>
>>
>> On 20.07.2017 10:49, Christoph Hellwig wrote:
>>> On Thu, Jul 20, 2017 at 10:47:50AM +0300, Nikolay Borisov wrote:
>>>>
>>>>
>>>> On 20.07.2017 10:47, Christoph Hellwig wrote:
>>>>> Odd.  And the patch to always just process a single extent from
>>>>> xfs_itruncate_extents fixes the issue for you, right?
>>>>
>>>> As far as I remember yes.
>>>
>>> Can you retest it under otherwise identical conditions?
>>
>> Okay, I confirm that indeed changing the define from 2 to 1 fixes the
>> issue.
> 
> But this is just on 4.4 and older, right?  We had issues in the
> allocation path there as well, which should be fixed in 4.11 and later,
> and in recrnt 4.9-stable releases.  Without those the allocation path
> might sometimes lock out of order as well.
> 

Couldn't repro on 4.11 and 4.10 (which supposedly shouldn't have the
fixes). I tried running generic/299 200 times on each kernel. Which
exactly are the patches in 4.11 which fix the alloc path?


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

* Re: [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi
  2017-07-28  6:05               ` Nikolay Borisov
@ 2017-07-31 12:09                 ` Christoph Hellwig
  0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2017-07-31 12:09 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Christoph Hellwig, linux-xfs

Hi Nikolay,

On Fri, Jul 28, 2017 at 09:05:51AM +0300, Nikolay Borisov wrote:
> Couldn't repro on 4.11 and 4.10 (which supposedly shouldn't have the
> fixes). I tried running generic/299 200 times on each kernel. Which
> exactly are the patches in 4.11 which fix the alloc path?

My suspects in 4.11 would be:

xfs: don't fail xfs_extent_busy allocation
xfs: improve handling of busy extents in the low-level allocator
xfs: fix len comparison in xfs_extent_busy_trim

4.10 also had a couple allocator fixes, but nothing that looks
directly related.

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

end of thread, other threads:[~2017-07-31 12:09 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-18 17:25 [PATCH, RFC] xfs: fix multi-AG deadlock in xfs_bunmapi Christoph Hellwig
2017-07-18 18:15 ` Darrick J. Wong
2017-07-18 18:27   ` Nikolay Borisov
2017-07-18 18:33     ` Darrick J. Wong
2017-07-19  7:36       ` Christoph Hellwig
2017-07-19 13:11 ` Nikolay Borisov
2017-07-19 15:09   ` Nikolay Borisov
2017-07-20  7:47     ` Christoph Hellwig
2017-07-20  7:47       ` Nikolay Borisov
2017-07-20  7:49         ` Christoph Hellwig
2017-07-20  7:51           ` Nikolay Borisov
2017-07-20 14:58           ` Nikolay Borisov
2017-07-21 10:26             ` Christoph Hellwig
2017-07-26 13:04             ` Christoph Hellwig
2017-07-26 13:59               ` Nikolay Borisov
2017-07-28  6:05               ` Nikolay Borisov
2017-07-31 12:09                 ` Christoph Hellwig

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).