[PATCH 4/9] nvme-core: code cleanup for __nvme_check_ready()

[Sagi Grimberg]

On 3/23/23 05:36, Chaitanya Kulkarni wrote:
> We can avoid using nested indentation by returning early if the
> controller is not fabrics. Additionally, instead of using comparisons,
> we can use a switch statement to determine if the required command is
> connect/auth_send/auth_recv. This will also help to reduce the length of
> the lines in the code and makes code easy to read since we don't have to
> verify three different comparisons. Lastly, fix the spelling of
> "appropriate" in the second comment.
> 
> Signed-off-by: Chaitanya Kulkarni <kch at nvidia.com>
> ---
>   drivers/nvme/host/core.c | 41 +++++++++++++++++++++++-----------------
>   1 file changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index c2fb7a0eba3d..a380938f5a06 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -721,25 +721,32 @@ bool __nvme_check_ready(struct nvme_ctrl *ctrl, struct request *rq,
>   	if (rq->q == ctrl->admin_q && (req->flags & NVME_REQ_USERCMD))
>   		return false;
>   
> -	if (ctrl->ops->flags & NVME_F_FABRICS) {
> -		/*
> -		 * Only allow commands on a live queue, except for the connect
> -		 * command, which is require to set the queue live in the
> -		 * appropinquate states.
> -		 */
> -		switch (ctrl->state) {
> -		case NVME_CTRL_CONNECTING:
> -			if (blk_rq_is_passthrough(rq) && nvme_is_fabrics(req->cmd) &&
> -			    (req->cmd->fabrics.fctype == nvme_fabrics_type_connect ||
> -			     req->cmd->fabrics.fctype == nvme_fabrics_type_auth_send ||
> -			     req->cmd->fabrics.fctype == nvme_fabrics_type_auth_receive))
> +	if (!(ctrl->ops->flags & NVME_F_FABRICS))
> +		return queue_live;
> +
> +	/*
> +	 * Only allow commands on a live queue, except for connect/auth_send/
> +	 * auth_recv commands which are require to set the queue live in the
> +	 * appropriate states.
> +	 */
> +	switch (ctrl->state) {
> +	case NVME_CTRL_CONNECTING:
> +		if (blk_rq_is_passthrough(rq) &&
> +		    nvme_is_fabrics(req->cmd)) {
> +			switch (req->cmd->fabrics.fctype) {
> +			case nvme_fabrics_type_connect:
> +			case nvme_fabrics_type_auth_send:
> +			case nvme_fabrics_type_auth_receive:
>   				return true;
> -			break;
> -		default:
> -			break;
> -		case NVME_CTRL_DEAD:
> -			return false;
> +			default:
> +				break;

I think we can safely remove the default case here.

> +			}
>   		}
> +		break;
> +	default:
> +		break;

While we're at it, same here.

> +	case NVME_CTRL_DEAD:
> +		return false;


>   	}
>   
>   	return queue_live;