Re: [PATCH 5/6] btrfs: Simplify extent type check

[David Sterba <dsterba@suse.cz>]

On Mon, Aug 05, 2019 at 05:47:07PM +0300, Nikolay Borisov wrote:
> Extent type can only be regular/prealloc/inline. The main branch of the
> 'if' already handles the first two, leaving the 'else' to handle inline.
> Furthermore, tree-checker ensures that leaf items are correct.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/inode.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 8e24b7641247..6c3f9f3a7ed1 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1502,18 +1502,14 @@ static noinline int run_delalloc_nocow(struct inode *inode,
>  			if (!btrfs_inc_nocow_writers(fs_info, disk_bytenr))
>  				goto out_check;
>  			nocow = true;
> -		} else if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> -			extent_end = found_key.offset +
> -				btrfs_file_extent_ram_bytes(leaf, fi);
> -			extent_end = ALIGN(extent_end,
> -					   fs_info->sectorsize);
> +		} else {
> +			extent_end = found_key.offset + ram_bytes;
> +			extent_end = ALIGN(extent_end, fs_info->sectorsize);
>  			/* Skip extents outside of our requested range */
>  			if (extent_end <= start) {
>  				path->slots[0]++;
>  				goto next_slot;
>  			}
> -		} else {
> -			BUG();

I am not sure if we should delete this or leave it (with a message what
happened). There are other places that switch value from a known set and
have a catch-all branch.

With your change the 'catch-all' is the inline extent type. It's true
that the checker should not let an unknown type appear in this code,
however I'd rather make it explicit that something is seriously wrong if
there's an unexpected type rather than silently continuing.

The BUG can be turned to actual error handling so we don't need to crash
at least.