| Message ID | 156944741018.300131.13838435268141846825.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs_scrub: fix IO error reporting | expand |
On 9/25/19 4:36 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > In xfs_iterate_filemaps, we query the number of bmaps for a given file > that we're going to iterate, so feed that information to bmap so that > the kernel won't waste time allocating in-kernel memory unnecessarily. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > scrub/filemap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > diff --git a/scrub/filemap.c b/scrub/filemap.c > index bdc6d8f9..aaaa0521 100644 > --- a/scrub/filemap.c > +++ b/scrub/filemap.c > @@ -71,7 +71,6 @@ xfs_iterate_filemaps( > map->bmv_length = ULLONG_MAX; > else > map->bmv_length = BTOBB(key->bm_length); > - map->bmv_count = BMAP_NR; > map->bmv_iflags = BMV_IF_NO_DMAPI_READ | BMV_IF_PREALLOC | > BMV_IF_NO_HOLES; > switch (whichfork) { > @@ -96,6 +95,7 @@ xfs_iterate_filemaps( > moveon = false; > goto out; > } > + map->bmv_count = min(fsx.fsx_nextents + 2, BMAP_NR); Was going to ask you to document the magical +2 here but IRC discussion suggests that it is in case fsx_nextents is 0, and we need to send in a count of at least 2 (header + 1 structure?) But if there are no extents, what are we trying to map in the loop below, anyway? > > while ((error = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) { > for (i = 0, p = &map[i + 1]; i < map->bmv_entries; i++, p++) { >
On Mon, Oct 21, 2019 at 01:05:06PM -0500, Eric Sandeen wrote: > On 9/25/19 4:36 PM, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > In xfs_iterate_filemaps, we query the number of bmaps for a given file > > that we're going to iterate, so feed that information to bmap so that > > the kernel won't waste time allocating in-kernel memory unnecessarily. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > scrub/filemap.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/scrub/filemap.c b/scrub/filemap.c > > index bdc6d8f9..aaaa0521 100644 > > --- a/scrub/filemap.c > > +++ b/scrub/filemap.c > > @@ -71,7 +71,6 @@ xfs_iterate_filemaps( > > map->bmv_length = ULLONG_MAX; > > else > > map->bmv_length = BTOBB(key->bm_length); > > - map->bmv_count = BMAP_NR; > > map->bmv_iflags = BMV_IF_NO_DMAPI_READ | BMV_IF_PREALLOC | > > BMV_IF_NO_HOLES; > > switch (whichfork) { > > @@ -96,6 +95,7 @@ xfs_iterate_filemaps( > > moveon = false; > > goto out; > > } > > + map->bmv_count = min(fsx.fsx_nextents + 2, BMAP_NR); > > Was going to ask you to document the magical +2 here but IRC discussion suggests > that it is in case fsx_nextents is 0, and we need to send in a count of at least > 2 (header + 1 structure?) > > But if there are no extents, what are we trying to map in the loop below, > anyway? Nothing. Will rework it with: if (fsx_nextents == 0) return 0; bmv_count = min(fsx_nextents + 1, BMAP_NR); while ((error = ioctl(...))) { --D > > > > while ((error = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) { > > for (i = 0, p = &map[i + 1]; i < map->bmv_entries; i++, p++) { > >
diff --git a/scrub/filemap.c b/scrub/filemap.c index bdc6d8f9..aaaa0521 100644 --- a/scrub/filemap.c +++ b/scrub/filemap.c @@ -71,7 +71,6 @@ xfs_iterate_filemaps( map->bmv_length = ULLONG_MAX; else map->bmv_length = BTOBB(key->bm_length); - map->bmv_count = BMAP_NR; map->bmv_iflags = BMV_IF_NO_DMAPI_READ | BMV_IF_PREALLOC | BMV_IF_NO_HOLES; switch (whichfork) { @@ -96,6 +95,7 @@ xfs_iterate_filemaps( moveon = false; goto out; } + map->bmv_count = min(fsx.fsx_nextents + 2, BMAP_NR); while ((error = ioctl(fd, XFS_IOC_GETBMAPX, map)) == 0) { for (i = 0, p = &map[i + 1]; i < map->bmv_entries; i++, p++) {