Received-SPF: pass (google.com: domain of linux-kernel+bounces-133522-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Andreas Dilger <adilger@dilger.ca>
Message-Id: <CBF9FEA5-0FD4-44BE-A2DC-54B7E5D1A874@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_23208463-7A57-4687-8755-FF47195EDFA6";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [PATCH v3 06/13] nilfs2: fiemap: return correct extent physical
 length
Date: Fri, 5 Apr 2024 13:26:37 -0600
In-Reply-To: <bd06389b4c9c33ab1411f2941875f02867b18642.1712126039.git.sweettea-kernel@dorminy.me>
Cc: Jonathan Corbet <corbet@lwn.net>,
 Kent Overstreet <kent.overstreet@linux.dev>,
 Brian Foster <bfoster@redhat.com>,
 Chris Mason <clm@fb.com>,
 Josef Bacik <josef@toxicpanda.com>,
 David Sterba <dsterba@suse.com>,
 Jaegeuk Kim <jaegeuk@kernel.org>,
 Chao Yu <chao@kernel.org>,
 Alexander Viro <viro@zeniv.linux.org.uk>,
 Christian Brauner <brauner@kernel.org>,
 Jan Kara <jack@suse.cz>,
 =?utf-8?Q?Micka=C3=ABl_Sala=C3=BCn?= <mic@digikod.net>,
 linux-doc@vger.kernel.org,
 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
 linux-bcachefs@vger.kernel.org,
 linux-btrfs <linux-btrfs@vger.kernel.org>,
 linux-f2fs-devel@lists.sourceforge.net,
 linux-fsdevel@vger.kernel.org,
 kernel-team@meta.com
To: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
References: <cover.1712126039.git.sweettea-kernel@dorminy.me>
 <bd06389b4c9c33ab1411f2941875f02867b18642.1712126039.git.sweettea-kernel@dorminy.me>


--Apple-Mail=_23208463-7A57-4687-8755-FF47195EDFA6
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy =
<sweettea-kernel@dorminy.me> wrote:
>=20
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> fs/nilfs2/inode.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>=20
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 4d3c347c982b..e3108f2cead7 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -1160,7 +1160,7 @@ int nilfs_fiemap(struct inode *inode, struct =
fiemap_extent_info *fieinfo,
> {
> 	struct the_nilfs *nilfs =3D inode->i_sb->s_fs_info;
> 	__u64 logical =3D 0, phys =3D 0, size =3D 0;
> -	__u32 flags =3D 0;
> +	__u32 flags =3D FIEMAP_EXTENT_HAS_PHYS_LEN;
>=20
> 	loff_t isize;
> 	sector_t blkoff, end_blkoff;
> 	sector_t delalloc_blkoff;
> @@ -1197,7 +1197,9 @@ int nilfs_fiemap(struct inode *inode, struct =
fiemap_extent_info *fieinfo,
> 			if (blkoff > end_blkoff)
> 				break;
>=20
> -			flags =3D FIEMAP_EXTENT_MERGED | =
FIEMAP_EXTENT_DELALLOC;
> +			flags =3D FIEMAP_EXTENT_MERGED |
> +				FIEMAP_EXTENT_DELALLOC |
> +				FIEMAP_EXTENT_HAS_PHYS_LEN;

IMHO, rather than setting "flags =3D FIEMAP..." here, it would be better =
to
initialize "flags |=3D FIEMAP_HAS_PHYS_LEN" right after =
fiemap_fill_next_extent()
is called, and use "flags |=3D FIEMAP_EXTENT_MERGED | =
FIEMAP_EXTENT_DELALLOC" here.

That makes it more clear that MERGED|DELALLOC are "add-on" flags beyond =
the
base flags, and if more flags are added in the future (e.g. COMPRESSED) =
then
the flag management will be simpler (more on this below).

> @@ -1261,14 +1263,16 @@ int nilfs_fiemap(struct inode *inode, struct =
fiemap_extent_info *fieinfo,
> 						break;
>=20
> 					/* Start another extent */
> -					flags =3D FIEMAP_EXTENT_MERGED;
> +					flags =3D FIEMAP_EXTENT_MERGED |
> +						=
FIEMAP_EXTENT_HAS_PHYS_LEN;

Strictly speaking, this new extent should not have FIEMAP_EXTENT_MERGED =
set,
and start out with only FIEMAP_EXTENT_HAS_PHYS_LEN, since it has not =
actually
been merged with anything.

> 					logical =3D blkoff << blkbits;
> 					phys =3D blkphy << blkbits;
> 					size =3D n << blkbits;
> 				}
> 			} else {
> 				/* Start a new extent */
> -				flags =3D FIEMAP_EXTENT_MERGED;
> +				flags =3D FIEMAP_EXTENT_MERGED |
> +					FIEMAP_EXTENT_HAS_PHYS_LEN;

Then this should be "flags |=3D FIEMAP_EXTENT_MERGED" only once a second
block has been merged into the prior one.

Cheers, Andreas


--Apple-Mail=_23208463-7A57-4687-8755-FF47195EDFA6
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAmYQUG0ACgkQcqXauRfM
H+DusBAAjLzaPC76LsfW44HKtdh7+jVJ1aHtau12SOTY37rSSphdiw/F9U6SbJ4b
PwbKUBwhMzFgGjXhyUE8AX8z7CmrggtnVzeRgw8ad+1O6HfDU6ibC1w04XREKgdZ
yVFCaBcijZA3b8POL+pTCS+URpbfc355WbiS1ioIiIa+1KLoBqe83dqfO9PVuS5V
BGUZh1pnUhpVxJOxppQ36NgweFjuEDvG32BpN9+NEZiQFrAj2GHP/QijiQTvNoPd
7noTlgPq8zcbGPPI6HTpVyBz3ZWxYmVQ5ImDZlAOUM4WLUoyVPbUjffKC1Y66ts+
FHQy/PRtvV2ohItJbfCTPTWjgBELQeONHyLnhbRoDYNGYyysyatprSq0TVpEVHN8
gukb731ZJzStKaIHIa+FtyW2DlzCfgPgwxT8LAr57xVPvwUFU/im8OztJdRSERf3
kkEg5t5fJsF+g/NekWV4g3ynWHNmKZ0dn40oanFNjHu6VsZj0Swzj9hOfRzYAm92
yr6hPj1VQY7N5tW90Z7sMiJgFQje8/tunjh2/MEweezooqTQ3hVQrvdRykwR4j00
ATlo/5vssOjOmRVThS0jxaSTSUUcW+nFxaEeJ00Cbm5AoRtLaGmpWBkvCVDqqavm
dKadcnyZvZ4mwK24TlrHA9EgHUCaGvNmEP6WJ4lfnieswdJIj60=
=J11M
-----END PGP SIGNATURE-----

--Apple-Mail=_23208463-7A57-4687-8755-FF47195EDFA6--