Received-SPF: pass (google.com: domain of linux-kernel+bounces-204057-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <d96c6e91-44f0-41c5-bebc-a092dc8a8406@oracle.com>
Date: Thu, 6 Jun 2024 10:50:34 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RFC v3 11/21] xfs: Unmap blocks according to forcealign
To: Dave Chinner <david@fromorbit.com>
Cc: djwong@kernel.org, hch@lst.de, viro@zeniv.linux.org.uk, brauner@kernel.org,
        jack@suse.cz, chandan.babu@oracle.com, willy@infradead.org,
        axboe@kernel.dk, martin.petersen@oracle.com,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        tytso@mit.edu, jbongio@google.com, ojaswin@linux.ibm.com,
        ritesh.list@gmail.com, mcgrof@kernel.org, p.raghav@samsung.com,
        linux-xfs@vger.kernel.org, catherine.hoang@oracle.com
References: <20240429174746.2132161-1-john.g.garry@oracle.com>
 <20240429174746.2132161-12-john.g.garry@oracle.com>
 <ZjGIktQV12qas14f@dread.disaster.area>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <ZjGIktQV12qas14f@dread.disaster.area>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?ekxqSWNPbWV3MHM1QTdBQUs3dkVIVnY0Q2F3UmVvZzVaN1d2ajN5b3FZQ2xt?=
 =?utf-8?B?UW1odXVEQ0hlMXphNlg0NkQxbGROS2RKK1doL1drekpzRHIyaDZhT1cwT2xY?=
 =?utf-8?B?S2FMWEU2K3ZuSGNIZEJ0WUV1dFhobVVCYU9CbWlwM083aElMWnZhRHFETWdG?=
 =?utf-8?B?T1VlSGFkdXJJQWVKMEQyajZTdzNTb05OanYzdk9LbmdZbGd0ZXZHTVF6VmIv?=
 =?utf-8?B?YUR6RXJBRksxRjlzQ1ZERTcyeHJkN3E3QUNZZitkcVN2SUU2WmNseDkzMFlJ?=
 =?utf-8?B?VHpkR2pHV3JTUzVzK0NxSzNPdFhLSFFMOUVSdW96WElOUzBZdCtpQnZmeDB1?=
 =?utf-8?B?dUlsUDZacm9vbmFlUHdaT2NNQkN6dElnd3JQYzRjVml0YVE0NEtWM3JvbFNB?=
 =?utf-8?B?eWtCTUJnR2dsK2NkbkZ1TklQbjI3Mk1Zem0xTGpEQVJiSnlDTm1hODJsdExV?=
 =?utf-8?B?aUZUL2ZBdEFsc0FrRnJvOHAvcW1Sd2R3MkZVY2RVZVlKOXpRRjkwWlhxc05p?=
 =?utf-8?B?YVMwcTRQTnNEUjRyODJGMHVFWnFiYzlsTkpKemN5TzM2RDVLQzdkZktEcHBO?=
 =?utf-8?B?ejFMRHlIRmJQR1AvTGRha2V0T0dhZ2xQZU5nYTlBcWs5MEVOQnFnWFJueEVY?=
 =?utf-8?B?bmF1NnlFcG5SVWVpcklFR054bXZXWWdiU0k2NW1WRTdYTkthSDR5eklwZE1V?=
 =?utf-8?B?MlpFL0dSd05yOFRjVVIwcFJ4c2VwVXUyc0ZKMFZHdU5XL0duTE41Y0NxWTZy?=
 =?utf-8?B?NkQxYngvRFl4R3kwZ25Xb3NGalF4NE8rQ21VY0JnK1ppUUpiRlpoNkZ0em1y?=
 =?utf-8?B?dHV4NkpyNm5pTkZieDBibVJ3Z04xOUM2V1g2ekIvV1gwWWdjcTB3Z0QvRzE3?=
 =?utf-8?B?aXFnaG5LSU43RnNWaVFubzZwekVSbHVBcEJVenl5YVg4TzB3emhuRVg3dXgr?=
 =?utf-8?B?T0ZSeGdmd1dHeU85ZnhQanVHUURCWkNhbFVZNG1VSjBuY0N0bU42T0xJcWt0?=
 =?utf-8?B?N0hvV1ZLd0E2SlcvaWpDMlMxd3dSTWpKZC96MXRaTWtlWUhPSGswYTZsM2NB?=
 =?utf-8?B?ZUp6Tk52MWpURjBpYXdPaExja3dPRVN3U3IrQjVES1hURURtT1grZ0J0V3BZ?=
 =?utf-8?B?N0llVVdEVFNDSUYxUEo2YmlCRThMUjRUN3c5aDhmNmNWWENLVG51ZjJLTVla?=
 =?utf-8?B?SjNRc2g4VkE5Z1UrOTBjSlhKSUJ3Qk56bFRFMVpPSUF2end1UEVrY3VpbEhj?=
 =?utf-8?B?bGxZdEEyS3M1WU9VRU9qMWhEcnY3Z0FLS1dHcTFvdjF1ejhuWWN1U1dQWmQz?=
 =?utf-8?B?ZkNKYStxSlZGS2FLdFJwcmdmcEtTaWxiKzQ1cndqU0JzT3NKQ3dPdWU5ekV3?=
 =?utf-8?B?QzR3Yk5nQXhWODdHVEtUS2x0QkREUlI0eVJnWnJUY3NEbjZtOERnOTFlZUY0?=
 =?utf-8?B?TTBNVDh1cFJLc0RLTG9XSVpNS0xmZWlScHVBS0x1Y3RjTjk4NkU0SmowOFA2?=
 =?utf-8?B?VU9kZGsxeU1SZkIva0JrRFVIQWxuUGlCZ1kvb0NZL2diekwrNWw2Z2hjL0sz?=
 =?utf-8?B?aFVrNTBkNkFPMlRwWCtuMlBvWVBuWWU5UVNNYm9EcThUWWREOStBZWdxM2pz?=
 =?utf-8?B?d3BITHV2ZHJ6Vlo1SGdYZ0dRaVg1OVdoWEtmQjZackVBbkR3UTlUYlZ2TmRB?=
 =?utf-8?B?RlpuMDRKV28vT2RCZ3F6blB6bnRpNFhXQUxEcWxPcWFYcmhWd3pUSXVRcHUz?=
 =?utf-8?B?N1BFR3NDRERnTXB5cDZZMnJJUndSUDNud1dUVWFUN2ZHK0xFOHF2U1RzdnNv?=
 =?utf-8?B?WitlMVd6V0ppTlNOU0tlazJnWVBnb2dZTjFYOGFUTU1qTUtxcUZjYmwzRFVU?=
 =?utf-8?B?VkxRbjZyOWlGVFRlYkQyZTJYZjVhbG5BVFRVK0x0K0NSZWM2ODNyL1NJWjZC?=
 =?utf-8?B?ZTRXZUFtVzlCbUN5ZVNoL1RXL1FWYUVtTnlvTXgwZUYvRTMyQXppUjVBT1My?=
 =?utf-8?B?RXozTGdDU1BUWFNJSUxNNlVDSTZOeWIvTVNwbTcrckM2R1pyc09EZFY1Q0cw?=
 =?utf-8?B?Zi9lMXA2NEh0ZE1qVzVieDh0UFhpYUd1S0VaZlRpQjZJL2YvODMxNERTSGgy?=
 =?utf-8?Q?ss9sM4faxb/zIx88XC4mgdZDx?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	vxrRHl4O9y3kSeqXQQllvH2eHIZmc3SaDXjcJnGhRDFkRPcszvf55t0DEH0JKJwf2nGkQ0Sew/F84+V5Y0MVkW7RsaWjOWbe8QReufYaL6I23b5VhiuvFxDsRiM/fUnkiDcZ6v4miNzsbPoQT15jsI3APt+zDl6AYPKNmbqT1TGhjfmfVN6jz8AvNFz3Pxr1V3mK2vtoaReZgLVJZnOUky2v58PFLKv5TxrajOh5fOyHgKj98eSrpLaHgF53+ZRemD6Z1htfWZssIxaqLbScZUnuH9EIq2R3c7vOGGibVf+O8nHhYmCboeyaJn+JduFrt9jnfRADcUYRMMaljtdVhhqNuYrdXVFoT6Y5vLh5VpK4dReGt344oYS0U7zKqWSUQLJgY+gBuvomSQCVAW2gQB6RaJL0grCMMgnu3FmiKGGjl05/sLXSmKWQYDwhJ5fHawjrov9VoITDcqqmpPW+dsn7s4KA0OxLb9/LKYL4Chw83KJT0WspoddWoo6nzPMp9HgmeVE40/RWJmRGzrbJeIhtuMNKbqWIhjJrd39LliZz3B/iBdGXBsPN1pTgXhuZcVW+PgBEueU5txsDYJPQ7G0DJZMhu3iXkmFNoNPNI1A=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1e35ac58-05a2-455c-e2cd-08dc860e2001
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jun 2024 09:50:40.6787
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ki1SnNKv7H9uq5AfETYeEgVs6QwgFd2gwW7AyMvrCbwnwiihum3NmaVOFjKTXkhR/lGXSNwh9MGta/uiV4Uciw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB4936
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16
 definitions=2024-06-06_01,2024-06-06_02,2024-05-17_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 bulkscore=0 spamscore=0
 malwarescore=0 suspectscore=0 phishscore=0 mlxlogscore=999 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2405010000
 definitions=main-2406060072
X-Proofpoint-GUID: lvVaajQCwM00l0NsRaQGDXUyzGLk8b4J
X-Proofpoint-ORIG-GUID: lvVaajQCwM00l0NsRaQGDXUyzGLk8b4J

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