Received-SPF: pass (google.com: domain of linux-ext4+bounces-2847-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <4c6da37f-c5ea-4008-8250-a75f3b65427e@oracle.com>
Date: Tue, 11 Jun 2024 11:08:13 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 13/22] xfs: Unmap blocks according to forcealign
To: dchinner@redhat.com
Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-btrfs@vger.kernel.org, linux-erofs@lists.ozlabs.org,
        linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net,
        linux-fsdevel@vger.kernel.org, gfs2@lists.linux.dev,
        linux-xfs@vger.kernel.org, catherine.hoang@oracle.com,
        ritesh.list@gmail.com, mcgrof@kernel.org,
        mikulas@artax.karlin.mff.cuni.cz, agruenba@redhat.com,
        miklos@szeredi.hu, martin.petersen@oracle.com, axboe@kernel.dk,
        tytso@mit.edu, viro@zeniv.linux.org.uk, brauner@kernel.org,
        jack@suse.com, chandan.babu@oracle.com, hch@lst.de, djwong@kernel.org
References: <20240607143919.2622319-1-john.g.garry@oracle.com>
 <20240607143919.2622319-14-john.g.garry@oracle.com>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20240607143919.2622319-14-john.g.garry@oracle.com>
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?L1pjMzkxR2dnUHlzS2R4bFlVdFFUSEdkNy93NlBWeVBaREcrWEhxdWxsSThN?=
 =?utf-8?B?T3dmSXpvVWdGSGNQbzZFVlBLazJUMVpMOWNwMk15NEJGQ0FVc2JIVHQrMWhB?=
 =?utf-8?B?QUJSd3FTMXQ5aHdHb211MTU0MVJlMUhkY1VudlgrWkRJZ2dOZWE1VkxaTnoy?=
 =?utf-8?B?ZFViYWhReEo5NEJ5OEtrMmMxRlJad3pUTGFLZUU4WXhic0RGWFM5SHNzMHFN?=
 =?utf-8?B?VG1QcE5VV0xHV3IySzJKZnMrV2c2azJzbWxRams0SjlWVklLUDhXMDFqazFV?=
 =?utf-8?B?Y2xHTGJTTUI3dVZ1czdneDBJWmorSk0xVXAwKzNZK2crQVJ2aFhVYzhCK3ZT?=
 =?utf-8?B?QVNLVVB3QmZXcEJpNnNmbUNraEg0cGZJREkyZ3JVY0FZQmFYNEZ4VFJ0R3Mv?=
 =?utf-8?B?d1BSZjlvOEUva2hPVVRGYlUrQ2NPOUNTTzFsM1N6TVRNdThLam92aFhHMEsr?=
 =?utf-8?B?dlhEUEZQQm9FOU9hSHBjajhvT2JDL2RLeGczQ3U1enQrY1puaVFQeVZ3eEpJ?=
 =?utf-8?B?YjlJVDNraVorNEhPQjFLNUdERHhRbFd2RHFjR0NJeWwzbUdrY3V4bW8wK0Vz?=
 =?utf-8?B?ZWx1dGxRNitmNlJ3ekZMaUNTRlR4dkUyM09WcENFU2lRY3p6TURBUmFhMkpt?=
 =?utf-8?B?SEhnKy9kRmNPdGc5TzdPSEFlTFlpUEpSU25VRVB3dmgxRmIxQTNZVDNMUmRj?=
 =?utf-8?B?YzZaT2VtTEdkUlVhZDljUzM4cFdDOFhtUWxPMk5RcHZvVjFuWWdZeHdXUS9C?=
 =?utf-8?B?cTl5d2lja3N6c1dhbG9hRjRZMnE4ckI3M013VGpVa2x5SGhjeXA4NEdHNS84?=
 =?utf-8?B?NW1wd1lOVFBqRGFidHBSek9obkw1T05hN2F2Q2hsRCt1TDMxWHhvWmp0S2dG?=
 =?utf-8?B?UDZiOVBWWXZCR3RUTVhuYUYwb01iTllHNm9pYnBhYkc2bUwrSlVYVW1lckRM?=
 =?utf-8?B?WVo4ek5pc0FDK0QrV2drTXB4alE5VzF1WUY2bzhMVkt0ajZyMitrNSs0UVRR?=
 =?utf-8?B?SCtWNUZQdEg0UDBWaDNCNU5TMCsxNzY2SE5PMEp4aEV4bis5UWt6Q3NTeGhp?=
 =?utf-8?B?Tlo1UURlOVd2V0h2TFhGd2svb1NSUVhUMmF4Wm4wdjJNZ2U3K29nMnlpYWRX?=
 =?utf-8?B?ditVbEFYNTlMNHREVTMxNlVRZENwMHQwTW9lOGFwZno2WVlOMk1SU2tTaXNh?=
 =?utf-8?B?NkFZUUpob2YwMGlPaDJrd2VIZnRDUWdNNVI0by9IbGdYWHpYKzVOWWFSVEV5?=
 =?utf-8?B?dUMrRTVFUE0wM1d5a0xRUVBwTC9aRWZXVXlCWHdGbzlzUHJacXNVb1R3NUph?=
 =?utf-8?B?S0hEZ0doTXg0aEtJd1pHZk90Rm9vOSt3U29OQVl2S3UwcnE3TlZUSzNKSFZV?=
 =?utf-8?B?YWhpd3ppSEhMa0w0RkJvUEdTa0lGR0NOMlJaQStnR21abzBONXl3akozWTBZ?=
 =?utf-8?B?Yzhyai9pcFVUYWRlMS9kSGtLckUrbHd0R2VrSWhzVHVNVW80YnFpc0o1RmQy?=
 =?utf-8?B?cEtVYVNtd1pJaFNGd0NoSy9hbGptZmtaVTFZOGJuTGNzdVlhMmpPWkZOdGRu?=
 =?utf-8?B?UFdDN3J2MjZIOUZ1OFV4czI0WWd4UTFWOVNhM1drY1J4ZEgrZmFnc1lldms5?=
 =?utf-8?B?OHhxaDkzSlhXc2QrSVRLMjNnUkp3a2RHZzVkZXY3MmxlZGI3OHEya24yRnFs?=
 =?utf-8?B?eFN6R1FTZFJLQ2FkNHZTcnlGem5QZ0U0R2ovbXdTWmZManZwYytSTlpLbHJv?=
 =?utf-8?B?TDdQMXhYTktXK3dSNW1UQm41dEE1YmNwRSs0WVIvV1ZmT09qUnR6djQ4T3FY?=
 =?utf-8?B?eHEzNjN0T3R0OHNjekZuT1VoM0ZsbW44NXQ0Tm5MM1V3Z2YvSTBZcitKVjVL?=
 =?utf-8?B?bndmWXVKejhTVHpwUTZTYXk4b0xmMHZVQ1liak1NK2xsYUNZNFNuM040MVdi?=
 =?utf-8?B?TjFIektHc3FNNEhNQXZtZ0pIYXcveEpvNzdYMGhBMU9XUkp2aEMxcFVkV3dN?=
 =?utf-8?B?Y3phUTBQczVEb0RZU1N5Rmtya3FaUllWeTNONVZVWDNVekJsQ2lva1pIZDhD?=
 =?utf-8?B?Q1p5dWp2ZU8xNDFxaWxvK29xa2NuZy95VW16a014SUUvUXh5TXlXS2k0WWQ4?=
 =?utf-8?B?NC9CSXI0SjU2Z0paQkkxQVVFTGFhRWRNVzJFSTRkWGIzbFFLdkpQVjJqVzB6?=
 =?utf-8?B?eWc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	PHIR7+kDiBA/C3E0yiqDs+3lmLGFVtYuBWX8m1ApbHT12IDWOmtt7xzFh1ZSVOF9gzFyJ17Rq2qN8w+URYiigj55f5eELp+7AjN6VBHoLq0Al4z9WchCz5dio2z6nllOKsPvgh8BZMhlrZ3RwdLXd+Tq50QguPLZc1RiljkRnFNbdtB42r4BfmVr1OFw1mGvc0L9QpMu5Zc/r8WUpg62fFJ8xSUw7rlaFF/eGwql3eKyt7WBJgoB9ZzKgMfGhWyehPoQXyeI1wob8ce/lYh5GxAoc0iF3Q4LHUI+k8ZlYvqV4IJux0mJkQOjY6vogomyjz+3zHEqtcCJ0245rE01y+/dzPJQn79TrpJCvl2veE3SexVn9Y7rBwgCT813hnSrApwoFePLTCpdaS6JqyyOk5Ee92V+iIZXMFVk83YEtVT4fGkaEiVOZPlgQPQO5DH+k3YZTnezZBr8qEnhoGKLgOxxU1GNwV67YT+PSAs1fzupswMDRllHfexwlKhqQkvHGic818+q4E7Md4uONIH3Bs7dv75pGSYnqQd9EuzcdZPF54O09DRS3GzGRpy7NXOhlVkjEUhCcfCDn4Grq963gqtmKrfz+qSdh6Elw0iqW20=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: af32a3c1-9c48-40ef-826f-08dc89fe6b9f
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jun 2024 10:08:20.2941
 (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: O9qaB3a67xwsxLy4yGn0n6oXZExP4sH+tpqTWM7bcPSq8i/YTWFwbpcnG2vUvNaGijF1KdvIrvbHANgNRU1Llw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4162
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-11_06,2024-06-11_01,2024-05-17_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 suspectscore=0
 phishscore=0 bulkscore=0 malwarescore=0 spamscore=0 mlxscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2405010000 definitions=main-2406110076
X-Proofpoint-ORIG-GUID: xQLl-Z5qdh4yrA8fvoEXImeQHFLOy4gr
X-Proofpoint-GUID: xQLl-Z5qdh4yrA8fvoEXImeQHFLOy4gr

On 07/06/2024 15:39, John Garry wrote:
> For when forcealign is enabled, blocks in an inode need to be unmapped
> according to extent alignment, like what is already done for rtvol.
> 
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>   fs/xfs/libxfs/xfs_bmap.c | 33 ++++++++++++++++++++++++++++-----
>   1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index c9cf138e13c4..2b6d5ebd8b4f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5380,6 +5380,20 @@ xfs_bmap_del_extent_real(
>   	return 0;
>   }
>   
> +static xfs_extlen_t
> +xfs_bunmapi_align(
> +	struct xfs_inode	*ip,
> +	xfs_fsblock_t		bno)
> +{
> +	if (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);
> +	}
> +	ASSERT(XFS_IS_REALTIME_INODE(ip));
> +	return xfs_rtb_to_rtxoff(ip->i_mount, bno);
> +}

This following updated version of xfs_bunmapi_align() seems to fix the 
issue reported in 
https://lore.kernel.org/linux-xfs/d96c6e91-44f0-41c5-bebc-a092dc8a8406@oracle.com/

static xfs_extlen_t
xfs_bunmapi_align(
         struct xfs_inode        *ip,
         xfs_fsblock_t           fsbno)
{
         if (xfs_inode_has_forcealign(ip)) {
                 struct xfs_mount        *mp = ip->i_mount;
                 xfs_agblock_t           agbno = XFS_FSB_TO_AGBNO(mp, 
fsbno);

                 if (is_power_of_2(ip->i_extsize))
                         return agbno & (ip->i_extsize - 1);
                 return do_div(agbno, ip->i_extsize);
         }
         ASSERT(XFS_IS_REALTIME_INODE(ip));
         return xfs_rtb_to_rtxoff(ip->i_mount, fsbno);
}


> +
>   /*
>    * Unmap (remove) blocks from a file.
>    * If nexts is nonzero then the number of extents to remove is limited to
> @@ -5402,6 +5416,7 @@ __xfs_bunmapi(
>   	struct xfs_bmbt_irec	got;		/* current extent record */
>   	struct xfs_ifork	*ifp;		/* inode fork pointer */
>   	int			isrt;		/* freeing in rt area */
> +	int			isforcealign;	/* freeing for inode with forcealign */
>   	int			logflags;	/* transaction logging flags */
>   	xfs_extlen_t		mod;		/* rt extent offset */
>   	struct xfs_mount	*mp = ip->i_mount;
> @@ -5439,6 +5454,8 @@ __xfs_bunmapi(
>   	}
>   	XFS_STATS_INC(mp, xs_blk_unmap);
>   	isrt = xfs_ifork_is_realtime(ip, whichfork);
> +	isforcealign = (whichfork != XFS_ATTR_FORK) &&
> +			xfs_inode_has_forcealign(ip);
>   	end = start + len;
>   
>   	if (!xfs_iext_lookup_extent_before(ip, ifp, &end, &icur, &got)) {
> @@ -5490,11 +5507,10 @@ __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);
> +		mod = xfs_bunmapi_align(ip, del.br_startblock + del.br_blockcount);
>   		if (mod) {
>   			/*
>   			 * Realtime extent not lined up at the end.
> @@ -5542,9 +5558,16 @@ __xfs_bunmapi(
>   			goto nodelete;
>   		}
>   
> -		mod = xfs_rtb_to_rtxoff(mp, del.br_startblock);
> +		mod = xfs_bunmapi_align(ip, del.br_startblock);
>   		if (mod) {
> -			xfs_extlen_t off = mp->m_sb.sb_rextsize - 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