Re: [PATCH RFC v3 11/21] xfs: Unmap blocks according to forcealign

[John Garry <john.g.garry@oracle.com>]

Hi Dave,

>>   
>>   	if (!xfs_iext_lookup_extent_before(ip, ifp, &end, &icur, &got)) {
>> @@ -5459,11 +5472,15 @@ __xfs_bunmapi(
>>   		if (del.br_startoff + del.br_blockcount > end + 1)
>>   			del.br_blockcount = end + 1 - del.br_startoff;
>>   
>> -		if (!isrt || (flags & XFS_BMAPI_REMAP))
>> +		if ((!isrt && !isforcealign) || (flags & XFS_BMAPI_REMAP))
>>   			goto delete;
>>   
>> -		mod = xfs_rtb_to_rtxoff(mp,
>> -				del.br_startblock + del.br_blockcount);
>> +		if (isrt)
>> +			mod = xfs_rtb_to_rtxoff(mp,
>> +					del.br_startblock + del.br_blockcount);
>> +		else if (isforcealign)
>> +			mod = xfs_forcealign_extent_offset(ip,
>> +					del.br_startblock + del.br_blockcount);
> There's got to be a cleaner way to do this.
> 
> We already know that either isrt or isforcealign must be set here,
> so there's no need for the "else if" construct.
> 
> Also, forcealign should take precedence over realtime, so that
> forcealign will work on realtime devices as well. I'd change this
> code to call a wrapper like:
> 
> 		mod = xfs_bunmapi_align(ip, del.br_startblock + del.br_blockcount);
> 
> static xfs_extlen_t
> xfs_bunmapi_align(
> 	struct xfs_inode	*ip,
> 	xfs_fsblock_t		bno)
> {
> 	if (!XFS_INODE_IS_REALTIME(ip)) {
> 		ASSERT(xfs_inode_has_forcealign(ip))
> 		if (is_power_of_2(ip->i_extsize))
> 			return bno & (ip->i_extsize - 1);
> 		return do_div(bno, ip->i_extsize);
> 	}
> 	return xfs_rtb_to_rtxoff(ip->i_mount, bno);
> }

I have made that change according to your suggestion.

However, now that the forcealign power-of-2 extsize restriction has been 
lifted, I am finding another bug.

I will just mention this now, as I want to go back to that other issue 
https://lore.kernel.org/linux-xfs/20240429174746.2132161-1-john.g.garry@oracle.com/T/#mebd7e97dfd0f12219bf92f289c41f62bf2abcff5

However this new one is pretty simple to reproduce.

We create a file:

ext:  logical_offset:        physical_offset: length:   expected: flags:
0:    0..    1775:      40200..     41975:   1776:            last,eof
/root/mnt2/file_22: 1 extent found

The forcealign extsize is 98304, i.e. 24 blks.

And then try to delete it, and get this:

[   17.604237] XFS: Assertion failed: tp->t_blk_res > 0, file: 
fs/xfs/libxfs/xfs_bmap.c, line: 5599
[   17.605908] ------------[ cut here ]------------
[   17.606884] kernel BUG at fs/xfs/xfs_message.c:102!
[   17.607917] Oops: invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[   17.609134] CPU: 3 PID: 240 Comm: kworker/3:2 Not tainted 
6.10.0-rc1-00096-g759a4497daa7-dirty #2553
[   17.610606] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org 04/01/2014
[   17.612619] Workqueue: xfs-inodegc/sda xfs_inodegc_worker
[   17.613682] RIP: 0010:assfail+0x36/0x40
[   17.614134] Code: c2 18 1e 1d 9d 48 89 f1 48 89 fe 48 c7 c7 43 f2 12 
9d e8 7d fd ff ff 80 3d ae 86 8d 01 00 75 09 90 0f 0b 90 c3 cc cc cc cc 
90 <0f> 0b 0f 1f 84 00 00 00 00 00 90 90 90 90 900
[   17.616478] RSP: 0018:ff4887cac0973c28 EFLAGS: 00010202
[   17.617080] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 
000000007fffffff
[   17.617899] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 
ffffffff9d12f243
[   17.618717] RBP: ff4887cac0973d40 R08: 0000000000000000 R09: 
000000000000000a
[   17.619548] R10: 000000000000000a R11: f000000000000000 R12: 
ff360ff881d88000[   17.620360] R13: 0000000000000000 R14: 
ff360ff8efa32040 R15: 000ffffffffe0000
[   17.620367] FS:  0000000000000000(0000) GS:ff360ffa75cc0000(0000) 
knlGS:0000000000000000
[   17.620369] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   17.620371] CR2: 00007f213b4ef008 CR3: 000000011cfca003 CR4: 
0000000000771ef0
[   17.620372] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
0000000000000000
[   17.620374] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 
0000000000000400
[   17.620375] PKRU: 55555554
[   17.620376] Call Trace:
[   17.620384]  <TASK>
[   17.624278]  ? die+0x32/0x90
[   17.624284]  ? do_trap+0xd8/0x100
[   17.624286]  ? assfail+0x36/0x40
[   17.624288]  ? do_error_trap+0x60/0x80
[   17.624289]  ? assfail+0x36/0x40
[   17.624292]  ? exc_invalid_op+0x53/0x70
[   17.628287]  ? assfail+0x36/0x40
[   17.628291]  ? asm_exc_invalid_op+0x1a/0x20
[   17.628295]  ? assfail+0x36/0x40
[   17.628297]  ? assfail+0x23/0x40
[   17.628299]  __xfs_bunmapi+0xb87/0xeb0
[   17.628304]  ? xfs_log_reserve+0x18f/0x210
[   17.629447]  xfs_bunmapi_range+0x62/0xd0
[   17.631699]  xfs_itruncate_extents_flags+0x1c4/0x410
[   17.631703]  xfs_inactive_truncate+0xba/0x140
[   17.631705]  xfs_inactive+0x331/0x3d0
[   17.631707]  xfs_inodegc_worker+0xb8/0x190
[   17.631709]  process_one_work+0x157/0x380
[   17.633949]  worker_thread+0x2ba/0x3e0
[   17.633953]  ? __pfx_worker_thread+0x10/0x10
[   17.633954]  kthread+0xce/0x100
[   17.633958]  ? __pfx_kthread+0x10/0x10
[   17.633960]  ret_from_fork+0x2c/0x50
[   17.633963]  ? __pfx_kthread+0x10/0x10
[   17.633965]  ret_from_fork_asm+0x1a/0x30
[   17.636314]  </TASK>
[   17.640377] Modules linked in:
[   17.642375] ---[ end trace 0000000000000000 ]---

Maybe something is going wrong with the AG bno vs fsbno indexing.

That extent allocated has fsbno=50552 (% 24 != 0). The agsize is 22416 fsb.

That 50552 comes from xfs_alloc_vextent_finish() with args->fsbno=50552 
= XFS_AGB_TO_FSB(mp, agno=1, agbno=17784) = 32K 
(=roundup_power_of_2(22416)) + 17784

So the agbno is aligned, but the fsbno is not.

In __xfs_bunmapi(), at this point:

	mod = xfs_bunmapi_align(ip, del.br_startblock);

	if (mod) {
		xfs_extlen_t off;

		if (isforcealign) {
			off = ip->i_extsize - mod;
		} else {
			ASSERT(isrt);
			off = mp->m_sb.sb_rextsize - mod;
		}

		/*
		 * Realtime extent is lined up at the end but not
		 * at the front.  We'll get rid of full extents if
		 * we can.
		 */

mod=8 del.br_startblock(48776) + del.br_blockcount(1776)=50552

Since this code was originally only used for rt, we may be missing 
setting some struct members which were being set for rt. For example, 
xfs_trans_alloc() accepts rtextents value, and maybe we should be doing 
similar for forcealign. Or xfs_fsb_to_db() has special RT handling, but 
I doubt this is the problem.

I have no such issue in using a power-of-2 extsize.

I do realise that I need to share the full code, but I'm reluctant to 
post with known bugs.

Please let me know if you have an idea. I'll look at this further when I 
get a chance.

Thanks,
John