linux-kernel - Re: [PATCH v4 13/22] xfs: Unmap blocks according to forcealign

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 11 Jun 2024 11:08:13 +0100
From: John Garry <john.g.garry@...cle.com>
To: dchinner@...hat.com
Cc: linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-btrfs@...r.kernel.org, linux-erofs@...ts.ozlabs.org,
        linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-fsdevel@...r.kernel.org, gfs2@...ts.linux.dev,
        linux-xfs@...r.kernel.org, catherine.hoang@...cle.com,
        ritesh.list@...il.com, mcgrof@...nel.org,
        mikulas@...ax.karlin.mff.cuni.cz, agruenba@...hat.com,
        miklos@...redi.hu, martin.petersen@...cle.com, axboe@...nel.dk,
        tytso@....edu, viro@...iv.linux.org.uk, brauner@...nel.org,
        jack@...e.com, chandan.babu@...cle.com, hch@....de, djwong@...nel.org
Subject: Re: [PATCH v4 13/22] xfs: Unmap blocks according to forcealign

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@...cle.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