Re: [PATCH v4 1/4] btrfs: extent_io: Do extra check for extent buffer read write functions

David Sterba
2020-08-13T14:06:09Z

On Wed, Aug 12, 2020 at 02:05:06PM +0800, Qu Wenruo wrote:
> +/*
> + * Check if the [start, start + len) range is valid before reading/writing
> + * the eb.
> + * NOTE: @start and @len are offset *INSIDE* the eb, *NOT* logical address.
> + *
> + * Caller should not touch the dst/src memory if this function returns error.
> + */
> +static int check_eb_range(const struct extent_buffer *eb, unsigned long start,
> + unsigned long len)
> +{
> + /* start, start + len should not go beyond eb->len nor overflow */
> + if (unlikely(start > eb->len || start + len > eb->len ||
> + len > eb->len)) {
> + btrfs_warn(eb->fs_info,
> +"btrfs: bad eb rw request, eb bytenr=%llu len=%lu rw start=%lu len=%lu\n",
> + eb->start, eb->len, start, len);
> + WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG));
> + return -EINVAL;
> + }
> + return 0;
> +}

This helper is similar to the check_setget_bounds that have some
performance impact,
https://lore.kernel.org/linux-btrfs/20200730110943.GE3703@twin.jikos.cz/
.

The extent buffer helpers are not called that often as the setget
helpers but still it could be improved to avoid the function call
penalty on the hot path.

static inline in check_eb_range(...) {
if (unlikely(out of range))
return report_eb_range(...)
return 0;
}

In the original code the range check was open coded and the above will
lead to the same asm output, while keeping the C code readable.