Re: [PATCH v2 05/14] block: blk-merge: fix to add the number of integrity segments to the request twice

[hch@xxxxxx]

The subject looks a bit odd, I think you're trying to say:

"do not add the number of integrity segments to the request twice"

based on the actual patch, is this correct?

> blk_integrity_merge_bio() not only performs conditional tests, but also
> updates the integrity segment information of request.
> It can be called twice when merging the bio into an existing request.
> 
> bio_attempt_bio_merge() or blk_mq_sched_try_merge()
>   blk_rq_merge_ok()
>     blk_integrity_merge_bio()  -  1
>   bio_attemp_{back|front}_merge()
>     ll_{back|front}_merge_fn()
>       ll_new_hw_segments()
>         blk_integrity_merge_bio()  -  2
> 
> The part of checking the conditions and the code to update the
> information of the actual request were separated. At this time, the
> ll_back_merge_fn was called by passth-path, so the condition check was
> called by all the separated functions.
> 
> And after success in blk_integrity_merge_bio(), the information of the
> request may be wrong if it is impossible to merge due to other
> conditional tests. Thus, it was changed to be called immediately before
> merging the bio's segments.


> +static inline bool blk_integrity_bypass_check(struct request *req,
> +					      struct bio *bio)
> +{
> +	return blk_integrity_rq(req) == 0 && bio_integrity(bio) == NULL;
> +}

No need for the explicit comparisms, this could just be:

	return !blk_integrity_rq(req) && !bio_integrity(bio);

and given that it just has two callers I'm not sure the helper is
all that useful to start with.

> +static bool __blk_integrity_mergeable(struct request_queue *q,
> +				      struct request *req, struct bio *bio)
> +{
> +	if (blk_integrity_rq(req) == 0 || bio_integrity(bio) == NULL)
> +		return false;
> +
> +	if (bio_integrity(req->bio)->bip_flags != bio_integrity(bio)->bip_flags)
> +		return false;
> +
> +	return true;
> +}
> +
> +bool blk_integrity_mergeable(struct request_queue *q, struct request *req,
> +			     struct bio *bio)
> +{
> +	if (blk_integrity_bypass_check(req, bio))
> +		return true;
> +
> +	return __blk_integrity_mergeable(q, req, bio);
> +}

Similarly here, I'm not even sure we need all these helpers.  I supect
the code would become more readable by dropping these helpers and just
making the checks explicitlẏ