[PATCH] nvme-fc: set max_segments to lldd max value

[Max Gurtovoy]

On 7/14/2020 10:03 PM, James Smart wrote:
> Currently the FC transport is set max_hw_sectors based on the lldds
> max sgl segment count. However, the block queue max segments is
> set based on the controller's max_segments count, which the transport
> does not set.  As such, the lldd is receiving sgl lists that are
> exceeding its max segment count.
>
> Set the controller max segment count and derive max_hw_sectors from
> the max segment count.
>
> Signed-off-by: James Smart <jsmart2021 at gmail.com>
> Reviewed-by: Ewan D. Milne <emilne at redhat.com>
> CC: Max Gurtovoy <maxg at mellanox.com>
>
> ---
> Looks like the setting of max_segments has been missing from all
> fabric transports for a while. Rdma had a fixup (ff13c1b87c97) from
> Max last fall that corrected this. But tcp and fc were still lacking.
> Copying Max to look at tcp.

The TCP stack is not limiting the IO size, I guess because of the fact 
it's a streaming protocol.

So I don't see a bug in the tcp driver but we can consider limiting the 
IO size if it will improve performance in some cases.

BTW, AFAIK the iscsi TCP acts the same.

Sagi,

do you see some benefits we can get by limiting the mdts for tcp ?


> ---
>   drivers/nvme/host/fc.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 6aa30bb5a762..e57e536546f7 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -3001,8 +3001,9 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
>   	if (ret)
>   		goto out_disconnect_admin_queue;
>   
> -	ctrl->ctrl.max_hw_sectors =
> -		(ctrl->lport->ops->max_sgl_segments - 1) << (PAGE_SHIFT - 9);
> +	ctrl->ctrl.max_segments = ctrl->lport->ops->max_sgl_segments;
> +	ctrl->ctrl.max_hw_sectors = ctrl->ctrl.max_segments <<
> +						(ilog2(SZ_4K) - 9);
>   
>   	blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
>   

Looks good,

Reviewed-by: Max Gurtovoy <maxg at mellanox.com>