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.
