Re: [PATCHv2] nvme: enable retries for authentication commands

[Christoph Hellwig <hch@lst.de>]

On Fri, Jan 26, 2024 at 07:37:09AM +0100, hare@kernel.org wrote:
>  #define nvme_auth_flags_from_qid(qid) \
> -	(qid == 0) ? 0 : BLK_MQ_REQ_NOWAIT | BLK_MQ_REQ_RESERVED
> +	(qid == 0) ? NVME_SUBMIT_RETRY : \
> +	NVME_SUBMIT_RETRY | NVME_SUBMIT_NOWAIT | NVME_SUBMIT_RESERVED
>  #define nvme_auth_queue_from_qid(ctrl, qid) \
>  	(qid == 0) ? (ctrl)->fabrics_q : (ctrl)->connect_q

Please just open code these two obfuscating macros with a single user
each to make the change easily reviewable.  That also means after that
you only nee to assign NVME_SUBMIT_RETRY once and not in both side of
a ternary operator.

>  	ret = __nvme_submit_sync_cmd(q, &cmd, NULL, data, data_len,
> -				     qid == 0 ? NVME_QID_ANY : qid,
> -				     0, flags);
> +				     qid == 0 ? NVME_QID_ANY : qid, flags);

And then split addition the calling convention change for the request
submission helpers from the actual behavior change into a well documented
prep patch as well.

> +enum {
> +	NVME_SUBMIT_AT_HEAD  = (__force nvme_submit_flags_t)(1 << 0),
> +	NVME_SUBMIT_NOWAIT = (__force nvme_submit_flags_t)(1 << 1),
> +	NVME_SUBMIT_RESERVED = (__force nvme_submit_flags_t)(1 << 2),
> +	NVME_SUBMIT_RETRY = (__force nvme_submit_flags_t)(1 << 3),

Maybe write some comments explaining the semantics while you're at it?