Re: [PATCH RFC v3 11/21] xfs: Unmap blocks according to forcealign
From: John Garry
Date: Thu Jun 06 2024 - 05:51:55 EST
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@xxxxxxxxxx/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