Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should use max_qb_init_rd_atom for initiator_depth? #497

Open
xhebox opened this issue Dec 29, 2024 · 4 comments
Open

should use max_qb_init_rd_atom for initiator_depth? #497

xhebox opened this issue Dec 29, 2024 · 4 comments

Comments

@xhebox
Copy link

xhebox commented Dec 29, 2024

linux-6.6.67/net/rds/ib.c
168-	rds_ibdev->max_8k_mrs = device->attrs.max_mr ?
169-		min_t(unsigned int, ((device->attrs.max_mr / 2) * RDS_MR_8K_SCALE),
170-		     rds_ib_mr_8k_pool_size) : rds_ib_mr_8k_pool_size;
171-
172:	rds_ibdev->max_initiator_depth = device->attrs.max_qp_init_rd_atom;
173-	rds_ibdev->max_responder_resources = device->attrs.max_qp_rd_atom;
174-
175-	rds_ibdev->vector_load = kcalloc(device->num_comp_vectors,
176-					sizeof(int),

linux-6.6.67/fs/smb/server/transport_rdma.c
1639-	u32 ird_ord_hdr[2];
1640-	int ret;
1641-
1642-	memset(&conn_param, 0, sizeof(conn_param));
1643:	conn_param.initiator_depth = min_t(u8, t->cm_id->device->attrs.max_qp_rd_atom,
1644-					  SMB_DIRECT_CM_INITIATOR_DEPTH);
1645-	conn_param.responder_resources = 0;
1646-
1647-	t->cm_id->device->ops.get_port_immutable(t->cm_id->device,

I am debugging some problems around ksmbd rdma. While this is unrelated, But I noticed that conn_param.initiator_depth is set to max_qb_rd_atom.

However all other modules seem set it to max_qb_init_rd_atom instead?

@namjaejeon
Copy link
Owner

You need to explain the difference between the two values ​​and say why they should change.

@xhebox
Copy link
Author

xhebox commented Dec 31, 2024

You need to explain the difference between the two values ​​and say why they should change.

I don't know clearly. I posted here for your help. I met some local length error and internet said that it may be related to initiator_depth or responder_resources

Later I noticed that, in ksmbd, initiator_depth = min(max_qp_rd_atom, xxx). While other code in kernel seems to set initiator_depth = min(max_qp_init_rd_atom, xxx). And according to man page:

responder_resources
    The maximum number of outstanding RDMA read and atomic operations that the local side will accept from the remote side. Applies only to RDMA_PS_TCP. This value must be less than or equal to the local RDMA device attribute max_qp_rd_atom, but preferably greater than or equal to the responder_resources value reported in the connect request event. 
initiator_depth
    The maximum number of outstanding RDMA read and atomic operations that the local side will have to the remote side. Applies only to RDMA_PS_TCP. This value must be less than or equal to the local RDMA device attribute max_qp_init_rd_atom and the initiator_depth value reported in the connect request event. 

               int                     max_qp_rd_atom;         /* Maximum number of RDMA Read & Atomic operations that can be outstanding per QP */
               ...
               int                     max_qp_init_rd_atom;    /* Maximum depth per QP for initiation of RDMA Read & Atomic operations */

max_qp_rd_atom is for responder_resources, and max_qp_init_rd_atom for initiator_depth.

This change did not solve my problem. I eventually upgraded my kernel to make it working. Maybe it is a bug of cifs client in the old kernel.

But this change also did not harm my setup, i.e. it seems correct. So, is it correct? I am not an expert of RDMA....

@namjaejeon
Copy link
Owner

This change did not solve my problem. I eventually upgraded my kernel to make it working. Maybe it is a bug of cifs client in the old kernel.

Yes, RDMA of cifs client is broken.

Okay, Can you make the patch and submit a patch to the mailing list ([email protected]) ?

@xhebox
Copy link
Author

xhebox commented Dec 31, 2024

Okay, Can you make the patch and submit a patch to the mailing list ([email protected]) ?

Sure, I'll try it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants