On Thu, Nov 14, 2024 at 04:33:49PM +0100, Christoph Hellwig wrote: > From: "Darrick J. Wong" <djwong@xxxxxxxxxx> > > In commit ca6448aed4f10a, we created an "end_daddr" variable to fix > fsmap reporting when the end of the range requested falls in the middle > of an unknown (aka free on the rmapbt) region. Unfortunately, I didn't > notice that the the code sets end_daddr to the last sector of the device > but then uses that quantity to compute the length of the synthesized > mapping. Er... I'm not sure why you're sending this patch to Hans and stable but not linux-xfs? Also it's already on the list, though nobody's responded to it yet: https://lore.kernel.org/linux-xfs/20241108173907.GB168069@frogsfrogsfrogs/ --D > Zizhi Wo later observed that when end_daddr isn't set, we still don't > report the last fsblock on a device because in that case (aka when > info->last is true), the info->high mapping that we pass to > xfs_getfsmap_group_helper has a startblock that points to the last > fsblock. This is also wrong because the code uses startblock to > compute the length of the synthesized mapping. > > Fix the second problem by setting end_daddr unconditionally, and fix the > first problem by setting start_daddr to one past the end of the range to > query. > > Cc: <stable@xxxxxxxxxxxxxxx> # v6.11 > Fixes: ca6448aed4f10a ("xfs: Fix missing interval for missing_owner in xfs fsmap") > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> > Reported-by: Zizhi Wo <wozizhi@xxxxxxxxxx> > --- > fs/xfs/xfs_fsmap.c | 35 +++++++++++++++++++++-------------- > 1 file changed, 21 insertions(+), 14 deletions(-) > > diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c > index 8d5d4d172d15..59b7a8e50414 100644 > --- a/fs/xfs/xfs_fsmap.c > +++ b/fs/xfs/xfs_fsmap.c > @@ -165,7 +165,8 @@ struct xfs_getfsmap_info { > xfs_daddr_t next_daddr; /* next daddr we expect */ > /* daddr of low fsmap key when we're using the rtbitmap */ > xfs_daddr_t low_daddr; > - xfs_daddr_t end_daddr; /* daddr of high fsmap key */ > + /* daddr of high fsmap key, or the last daddr on the device */ > + xfs_daddr_t end_daddr; > u64 missing_owner; /* owner of holes */ > u32 dev; /* device id */ > /* > @@ -388,8 +389,8 @@ xfs_getfsmap_group_helper( > * we calculated from userspace's high key to synthesize the record. > * Note that if the btree query found a mapping, there won't be a gap. > */ > - if (info->last && info->end_daddr != XFS_BUF_DADDR_NULL) > - frec->start_daddr = info->end_daddr; > + if (info->last) > + frec->start_daddr = info->end_daddr + 1; > else > frec->start_daddr = xfs_gbno_to_daddr(xg, startblock); > > @@ -737,8 +738,8 @@ xfs_getfsmap_rtdev_rtbitmap_helper( > * we calculated from userspace's high key to synthesize the record. > * Note that if the btree query found a mapping, there won't be a gap. > */ > - if (info->last && info->end_daddr != XFS_BUF_DADDR_NULL) { > - frec.start_daddr = info->end_daddr; > + if (info->last) { > + frec.start_daddr = info->end_daddr + 1; > } else { > frec.start_daddr = xfs_rtb_to_daddr(mp, start_rtb); > } > @@ -1108,7 +1109,10 @@ xfs_getfsmap( > struct xfs_trans *tp = NULL; > struct xfs_fsmap dkeys[2]; /* per-dev keys */ > struct xfs_getfsmap_dev handlers[XFS_GETFSMAP_DEVS]; > - struct xfs_getfsmap_info info = { NULL }; > + struct xfs_getfsmap_info info = { > + .fsmap_recs = fsmap_recs, > + .head = head, > + }; > bool use_rmap; > int i; > int error = 0; > @@ -1185,9 +1189,6 @@ xfs_getfsmap( > > info.next_daddr = head->fmh_keys[0].fmr_physical + > head->fmh_keys[0].fmr_length; > - info.end_daddr = XFS_BUF_DADDR_NULL; > - info.fsmap_recs = fsmap_recs; > - info.head = head; > > /* For each device we support... */ > for (i = 0; i < XFS_GETFSMAP_DEVS; i++) { > @@ -1200,17 +1201,23 @@ xfs_getfsmap( > break; > > /* > - * If this device number matches the high key, we have > - * to pass the high key to the handler to limit the > - * query results. If the device number exceeds the > - * low key, zero out the low key so that we get > - * everything from the beginning. > + * If this device number matches the high key, we have to pass > + * the high key to the handler to limit the query results, and > + * set the end_daddr so that we can synthesize records at the > + * end of the query range or device. > */ > if (handlers[i].dev == head->fmh_keys[1].fmr_device) { > dkeys[1] = head->fmh_keys[1]; > info.end_daddr = min(handlers[i].nr_sectors - 1, > dkeys[1].fmr_physical); > + } else { > + info.end_daddr = handlers[i].nr_sectors - 1; > } > + > + /* > + * If the device number exceeds the low key, zero out the low > + * key so that we get everything from the beginning. > + */ > if (handlers[i].dev > head->fmh_keys[0].fmr_device) > memset(&dkeys[0], 0, sizeof(struct xfs_fsmap)); > > -- > 2.45.2 >
