Re: [PATCH 2/9] block: add scatterlist-less DMA mapping helpers

[Daniel Gomez <da.gomez@xxxxxxxxxx>]

On 16/06/2025 13.31, Christoph Hellwig wrote:
> On Mon, Jun 16, 2025 at 08:43:22AM +0200, Daniel Gomez wrote:
>>>> This comment does not match with blk_rq_dma_map_iter_start(). It returns false
>>>> and status is BLK_STS_INVAL.
>>>
>>> I went over you comment a few times and still don't understand it.
>>
>> The way I read the comment is that status is only valid when
>> blk_rq_dma_map_iter_* returns false.
> 
> Yes.
> 
>> But blk_rq_dma_map_iter_start() can return false and an invalid status (in the
>> default switch case).
> 
> The status field is valid.  Your patch below leaves it uninitialized
> instead, which leaves me even more confused than the previous comment.
> 

status is initialized at line 160 after memset():

	iter->status = BLK_STS_OK;

So, what the patch does is (for blk_rq_dma_map_iter_start()):

1. Initialize status to valid (line 160)

	iter->status = BLK_STS_OK;
	
2. In the if/switch case default case (line 181-183):
	It sets the status to invalid and *iter_start() returns false:
	
		default:
			iter->status = BLK_STS_INVAL;
			return false;


Removing the invalid assignment "makes it" valid (because of the initialization)
and also matches with the blk_dma_iter data struct comment (lines 13-14 in
blk-mq-dma.h):

	/* Status code. Only valid when blk_rq_dma_map_iter_* returned false */
	blk_status_t			status;