Re: [PATCH 5/9] bio-integrity: fold bio_integrity_enabled to bio_integrity_prep

From: Christoph Hellwig
Date: Thu Apr 06 2017 - 07:44:01 EST


> --- a/Documentation/block/data-integrity.txt
> +++ b/Documentation/block/data-integrity.txt
> @@ -202,9 +202,6 @@ will require extra work due to the application tag.
> added. It is up to the caller to ensure that the bio does not
> change while I/O is in progress.
>
> - bio_integrity_prep() should only be called if
> - bio_integrity_enabled() returned 1.
> -

This should grow a blurb that bio_integrity_prep will complete the bio
if an error occurs.

> /**
> - * bio_integrity_prep - Prepare bio for integrity I/O
> + * bio_integrity_setup - Prepare bio for integrity I/O
> * @bio: bio to prepare
> *
> * Description: Allocates a buffer for integrity metadata, maps the
> @@ -269,7 +231,7 @@ static int bio_integrity_process(struct bio *bio,
> * block device's integrity function. In the READ case, the buffer
> * will be prepared for DMA and a suitable end_io handler set up.
> */
> -int bio_integrity_prep(struct bio *bio)
> +static int bio_integrity_setup(struct bio *bio)

Instead of renaming the function I'd suggest simply merging the two,
the end result will be smaller and more readable.

> + if (bio_data_dir(bio) == READ && bi->profile->verify_fn != NULL &&
> + (bi->flags & BLK_INTEGRITY_VERIFY))
> + goto need_prep;
> +
> + if (bio_data_dir(bio) == WRITE && bi->profile->generate_fn != NULL &&
> + (bi->flags & BLK_INTEGRITY_GENERATE))
> + goto need_prep;

I'd invert the conditions to avoid the goto:

if (bio_data_dir(bio) == READ) {
if (!bi->profile->verify_fn ||
!(bi->flags & BLK_INTEGRITY_VERIFY))
return 0;
} else {
if (!bi->profile->generate_fn ||
!(bi->flags & BLK_INTEGRITY_GENERATE))
return 0;
}