Re: [PATCH] block: t10-pi: Return correct ref tag when queue has no integrity profile

[Christoph Hellwig <hch@lst.de>]

On Thu, Jul 04, 2024 at 11:45:15AM +0530, Kanchan Joshi wrote:
> From: Anuj Gupta <anuj20.g@samsung.com>
> 
> Commit <c6e56cf6b2e7> (block: move integrity information into
> queue_limits) changed the ref tag calculation logic. It would break if
> there is no integrity profile. This in turn causes read/write failures
> for such cases.

Can you explain the scenario a bit better?  I guess this is for when
the drivers use PRACT to insert/strip PI because BLK_DEV_INTEGRITY
is disabled?

> 
> Fixes: <c6e56cf6b2e7> (block: move integrity information into queue_limits)

This is not the standard formatting for fixes tags.

>  
>  static inline u32 t10_pi_ref_tag(struct request *rq)
>  {
> -	unsigned int shift = rq->q->limits.integrity.interval_exp;
> +	unsigned int shift = ilog2(queue_logical_block_size(rq->q));
>  
> +	if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) &&
> +	    rq->q->limits.integrity.interval_exp)
> +		shift = rq->q->limits.integrity.interval_exp;
>  	return blk_rq_pos(rq) >> (shift - SECTOR_SHIFT) & 0xffffffff;

But this only works when the interval_exp equals the block size.

So I think the proper fix that not only addresses the regression, but
also the long standing buf for larger interval_exp is to make sure
interval_exp is always initialized, including for
!CONFIG_BLK_DEV_INTEGRITY.