Skip to content

Commit 49edd5b

Browse files
Andreas GruenbacherAstralBob
authored andcommitted
gfs2: Fixes to "Implement iomap for block_map"
It turns out that commit 3974320 "Implement iomap for block_map" introduced a few bugs that trigger occasional failures with xfstest generic/476: In gfs2_iomap_begin, we jump to do_alloc when we determine that we are beyond the end of the allocated metadata (height > ip->i_height). There, we can end up calling hole_size with a metapath that doesn't match the current metadata tree, which doesn't make sense. After untangling the code at do_alloc, fix this by checking if the block we are looking for is within the range of allocated metadata. In addition, add a BUG() in case gfs2_iomap_begin is accidentally called for reading stuffed files: this is handled separately. Make sure we don't truncate iomap->length for reads beyond the end of the file; in that case, the entire range counts as a hole. Finally, revert to taking a bitmap write lock when doing allocations. It's unclear why that change didn't lead to any failures during testing. Signed-off-by: Andreas Gruenbacher <[email protected]> Signed-off-by: Bob Peterson <[email protected]>
1 parent 3527799 commit 49edd5b

File tree

1 file changed

+23
-20
lines changed

1 file changed

+23
-20
lines changed

fs/gfs2/bmap.c

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
716716
__be64 *ptr;
717717
sector_t lblock;
718718
sector_t lend;
719-
int ret;
719+
int ret = 0;
720720
int eob;
721721
unsigned int len;
722722
struct buffer_head *bh;
@@ -728,12 +728,14 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
728728
goto out;
729729
}
730730

731-
if ((flags & IOMAP_REPORT) && gfs2_is_stuffed(ip)) {
732-
gfs2_stuffed_iomap(inode, iomap);
733-
if (pos >= iomap->length)
734-
return -ENOENT;
735-
ret = 0;
736-
goto out;
731+
if (gfs2_is_stuffed(ip)) {
732+
if (flags & IOMAP_REPORT) {
733+
gfs2_stuffed_iomap(inode, iomap);
734+
if (pos >= iomap->length)
735+
ret = -ENOENT;
736+
goto out;
737+
}
738+
BUG_ON(!(flags & IOMAP_WRITE));
737739
}
738740

739741
lblock = pos >> inode->i_blkbits;
@@ -744,7 +746,7 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
744746
iomap->type = IOMAP_HOLE;
745747
iomap->length = (u64)(lend - lblock) << inode->i_blkbits;
746748
iomap->flags = IOMAP_F_MERGED;
747-
bmap_lock(ip, 0);
749+
bmap_lock(ip, flags & IOMAP_WRITE);
748750

749751
/*
750752
* Directory data blocks have a struct gfs2_meta_header header, so the
@@ -787,27 +789,28 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
787789
iomap->flags |= IOMAP_F_BOUNDARY;
788790
iomap->length = (u64)len << inode->i_blkbits;
789791

790-
ret = 0;
791-
792792
out_release:
793793
release_metapath(&mp);
794-
bmap_unlock(ip, 0);
794+
bmap_unlock(ip, flags & IOMAP_WRITE);
795795
out:
796796
trace_gfs2_iomap_end(ip, iomap, ret);
797797
return ret;
798798

799799
do_alloc:
800-
if (!(flags & IOMAP_WRITE)) {
801-
if (pos >= i_size_read(inode)) {
800+
if (flags & IOMAP_WRITE) {
801+
ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
802+
} else if (flags & IOMAP_REPORT) {
803+
loff_t size = i_size_read(inode);
804+
if (pos >= size)
802805
ret = -ENOENT;
803-
goto out_release;
804-
}
805-
ret = 0;
806-
iomap->length = hole_size(inode, lblock, &mp);
807-
goto out_release;
806+
else if (height <= ip->i_height)
807+
iomap->length = hole_size(inode, lblock, &mp);
808+
else
809+
iomap->length = size - pos;
810+
} else {
811+
if (height <= ip->i_height)
812+
iomap->length = hole_size(inode, lblock, &mp);
808813
}
809-
810-
ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
811814
goto out_release;
812815
}
813816

0 commit comments

Comments
 (0)