Messages in this thread | | | From | Johannes Thumshirn <> | Subject | Re: [PATCH] fs: btrfs: extent_map: removed unneeded variable | Date | Wed, 30 Jun 2021 11:17:30 +0000 |
| |
On 30/06/2021 12:01, David Sterba wrote: > On Tue, Jun 29, 2021 at 09:04:40AM +0000, Johannes Thumshirn wrote: >> On 29/06/2021 10:51, lijian_8010a29@163.com wrote: >>> From: lijian <lijian@yulong.com> >>> >>> removed unneeded variable 'ret'. >> >> Wouldn't it make more sense to return an error (-ENOENT??) in case >> the em lookup fails and handle the error in btrfs_finish_ordered_io() >> as this is the only caller of unpin_extent_cache()? >> >> I've actually tripped over this a couple of times already when >> investigating extent map and ordered extent splitting problems >> on zoned filesystems: >> >> em = lookup_extent_mapping(tree, start, len); >> WARN_ON(!em || em->start != start); >> >> Maybe even turn this WARN_ON() into an ASSERT() when introducing proper >> error handling, as we shouldn't really get there unless we have a logical >> error. > > If you have real workloads hitting the warning then it really should be > proper error handling, not even an assert. >
Up to now it's been coding errors from my side so I think it warrants an ASSERT().
But still we should handle the error in the caller.
| |