diff options
author | Mark Fasheh <mfasheh@suse.de> | 2015-05-19 15:49:50 -0400 |
---|---|---|
committer | Chris Mason <clm@fb.com> | 2015-05-19 21:04:17 -0400 |
commit | 2c2ed5aa0154c0be67f98c970de6b5587dbc045a (patch) | |
tree | 93a9be4a974851d8a164b621711f3721d5966d5d /fs | |
parent | 062c19e9dd692b8a78e3532f71c290520a2ab437 (diff) |
btrfs: clear 'ret' in btrfs_check_shared() loop
btrfs_check_shared() is leaking a return value of '1' from
find_parent_nodes(). As a result, callers (in this case, extent_fiemap())
are told extents are shared when they are not. This in turn broke fiemap on
btrfs for kernels v3.18 and up.
The fix is simple - we just have to clear 'ret' after we are done processing
the results of find_parent_nodes().
It wasn't clear to me at first what was happening with return values in
btrfs_check_shared() and find_parent_nodes() - thanks to Josef for the help
on irc. I added documentation to both functions to make things more clear
for the next hacker who might come across them.
If we could queue this up for -stable too that would be great.
Signed-off-by: Mark Fasheh <mfasheh@suse.de>
Reviewed-by: Josef Bacik <jbacik@fb.com>
Signed-off-by: Chris Mason <clm@fb.com>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/btrfs/backref.c | 17 |
1 files changed, 17 insertions, 0 deletions
diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c index 9de772ee0031..614aaa1969bd 100644 --- a/fs/btrfs/backref.c +++ b/fs/btrfs/backref.c | |||
@@ -880,6 +880,8 @@ static int __add_keyed_refs(struct btrfs_fs_info *fs_info, | |||
880 | * indirect refs to their parent bytenr. | 880 | * indirect refs to their parent bytenr. |
881 | * When roots are found, they're added to the roots list | 881 | * When roots are found, they're added to the roots list |
882 | * | 882 | * |
883 | * NOTE: This can return values > 0 | ||
884 | * | ||
883 | * FIXME some caching might speed things up | 885 | * FIXME some caching might speed things up |
884 | */ | 886 | */ |
885 | static int find_parent_nodes(struct btrfs_trans_handle *trans, | 887 | static int find_parent_nodes(struct btrfs_trans_handle *trans, |
@@ -1198,6 +1200,19 @@ int btrfs_find_all_roots(struct btrfs_trans_handle *trans, | |||
1198 | return ret; | 1200 | return ret; |
1199 | } | 1201 | } |
1200 | 1202 | ||
1203 | /** | ||
1204 | * btrfs_check_shared - tell us whether an extent is shared | ||
1205 | * | ||
1206 | * @trans: optional trans handle | ||
1207 | * | ||
1208 | * btrfs_check_shared uses the backref walking code but will short | ||
1209 | * circuit as soon as it finds a root or inode that doesn't match the | ||
1210 | * one passed in. This provides a significant performance benefit for | ||
1211 | * callers (such as fiemap) which want to know whether the extent is | ||
1212 | * shared but do not need a ref count. | ||
1213 | * | ||
1214 | * Return: 0 if extent is not shared, 1 if it is shared, < 0 on error. | ||
1215 | */ | ||
1201 | int btrfs_check_shared(struct btrfs_trans_handle *trans, | 1216 | int btrfs_check_shared(struct btrfs_trans_handle *trans, |
1202 | struct btrfs_fs_info *fs_info, u64 root_objectid, | 1217 | struct btrfs_fs_info *fs_info, u64 root_objectid, |
1203 | u64 inum, u64 bytenr) | 1218 | u64 inum, u64 bytenr) |
@@ -1226,11 +1241,13 @@ int btrfs_check_shared(struct btrfs_trans_handle *trans, | |||
1226 | ret = find_parent_nodes(trans, fs_info, bytenr, elem.seq, tmp, | 1241 | ret = find_parent_nodes(trans, fs_info, bytenr, elem.seq, tmp, |
1227 | roots, NULL, root_objectid, inum); | 1242 | roots, NULL, root_objectid, inum); |
1228 | if (ret == BACKREF_FOUND_SHARED) { | 1243 | if (ret == BACKREF_FOUND_SHARED) { |
1244 | /* this is the only condition under which we return 1 */ | ||
1229 | ret = 1; | 1245 | ret = 1; |
1230 | break; | 1246 | break; |
1231 | } | 1247 | } |
1232 | if (ret < 0 && ret != -ENOENT) | 1248 | if (ret < 0 && ret != -ENOENT) |
1233 | break; | 1249 | break; |
1250 | ret = 0; | ||
1234 | node = ulist_next(tmp, &uiter); | 1251 | node = ulist_next(tmp, &uiter); |
1235 | if (!node) | 1252 | if (!node) |
1236 | break; | 1253 | break; |