Re: [PATCH v2 1/3] block: fix arg type of bio_trim()

[Christoph Hellwig <hch@infradead.org>]

On Wed, Jul 21, 2021 at 03:26:58PM +0900, Naohiro Aota wrote:
> +void bio_trim(struct bio *bio, sector_t offset, sector_t size)
>  {
> +	const sector_t uint_max_sectors = UINT_MAX >> SECTOR_SHIFT;

I'd make this a #define and keep it close to the struct bio definition
named something like BIO_MAX_SECTORS

> +
> +	/*
> +	 * 'bio' is a cloned bio which we need to trim to match the given
> +	 * offset and size.
>  	 */

This should go into the kernel doc comment.

Something like:

/**
 * bio_trim - trim a bio to the given offset and size
 * @bio:        bio to trim
 * @offset:     number of sectors to trim from the front of @bio
 * @size:       size we want to trim @bio to, in sectors
 *
 * This function is typically used for bios that are cloned and
 * submitted to the underlying device in parts.
 */


> +	/* sanity check */
> +	if (WARN_ON(offset > uint_max_sectors || size > uint_max_sectors ||
> +		    offset + size > bio->bi_iter.bi_size))
> +		return;

No real need for the comment. WARN_ON pretty much implies a sanity
check.  I'd make this a WARN_ON_ONCE as otherwise you'll probably
drown in these warnings if they ever hit.

> -extern void bio_trim(struct bio *bio, int offset, int size);
> +extern void bio_trim(struct bio *bio, sector_t offset, sector_t size);

Please drop the extern while you're at it.