Hi Sagi, Thanks for your feedback. I will update the patch according to your remarks and send a new version soon. Regards, Sergey > -----Original Message----- > From: Sagi Grimberg [mailto:sagi@xxxxxxxxxxx] > Sent: Monday, April 16, 2018 11:57 AM > To: Sergey Gorenko <sergeygo@xxxxxxxxxxxx>; linux- > rdma@xxxxxxxxxxxxxxx; Jason Gunthorpe <jgg@xxxxxxxxxxxx>; > dledford@xxxxxxxxxx > Cc: Vladimir Neyelov <vladimirn@xxxxxxxxxxxx> > Subject: Re: [PATCH] IB/iser: Fix RNR errors > > Hi Sergey, sorry for the late response, missed this patch. > > On 03/13/2018 05:25 PM, Sergey Gorenko wrote: > > Some users complain about RNR errors on the target, when heavy > > high-priority tasks run on the initiator. After the investigation, we > > found out that the receive WRs were exhausted, because the initiator > > could not post them on time. > > > > Receive work reqeusts are posted in chunks of min_posted_rx to reduce > > the number of hits to the HCA. The WRs are posted in the receive > > completion handler when the number of free receive buffers reaches > > min_posted_rx. But on a high-loaded host, receive CQEs processing can > > be delayed and all receive WRs will be exhausted. > > In this case, the target will get an RNR error. > > > > To avoid this, we post receive WR, as soon as at least one receive > > buffer is freed. This increases the number of hits to the HCA, but > > test results show that performance degradation is not significant. > > Overall this looks fine, small comments below. > > > @@ -322,30 +319,28 @@ void iser_free_rx_descriptors(struct iser_conn > *iser_conn) > > static int iser_post_rx_bufs(struct iscsi_conn *conn, struct iscsi_hdr *req) > > { > > struct iser_conn *iser_conn = conn->dd_data; > > - struct ib_conn *ib_conn = &iser_conn->ib_conn; > > struct iscsi_session *session = conn->session; > > + /* There is one posted recv buffer (for the last login response). */ > > + int already_posted = 1; > > No need for a local variable for a constant really. > > > + int i; > > > > iser_dbg("req op %x flags %x\n", req->opcode, req->flags); > > /* check if this is the last login - going to full feature phase */ > > if ((req->flags & ISCSI_FULL_FEATURE_PHASE) != > ISCSI_FULL_FEATURE_PHASE) > > return 0; > > > > - /* > > - * Check that there is one posted recv buffer > > - * (for the last login response). > > - */ > > - WARN_ON(ib_conn->post_recv_buf_count != 1); > > - > > if (session->discovery_sess) { > > iser_info("Discovery session, re-using login RX buffer\n"); > > return 0; > > } else > > iser_info("Normal session, posting batch of RX %d > buffers\n", > > - iser_conn->min_posted_rx); > > + iser_conn->qp_max_recv_dtos - already_posted); > > > > /* Initial post receive buffers */ > > - if (iser_post_recvm(iser_conn, iser_conn->min_posted_rx)) > > - return -ENOMEM; > > + for (i = already_posted; i < iser_conn->qp_max_recv_dtos; i++) { > > + if (iser_post_recvm(iser_conn, &iser_conn->rx_descs[i])) > > + return -ENOMEM; > > + } > > Please propagate the return code here. ��.n��������+%������w��{.n�����{���fk��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
