| Message ID | 570548AB.4090802@grimberg.me (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Apr 06, 2016 at 08:34:35PM +0300, sagig wrote: > So this scatterlist seems perfectly fine (no gaps). The only thing I > can think of is that we don't have enough space reserved, but I counted > 127 pages in the scatterlist when iser reserves 128 (by default). > > Just in case I mis-counted, can you (or Max) try with the below > patch: This works fine for me, but I wonder if the miscounting is in the block layer? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> So this scatterlist seems perfectly fine (no gaps). The only thing I >> can think of is that we don't have enough space reserved, but I counted >> 127 pages in the scatterlist when iser reserves 128 (by default). >> >> Just in case I mis-counted, can you (or Max) try with the below >> patch: > > This works fine for me, but I wonder if the miscounting is in the > block layer? We communicate the "max_pages" via sg_tablesize and max_hw_sectors. iSER communicates (or at least supposed to) sg_tablesize=128 and max_sectors=1024 to scsi (which sets the block queues attributes), that should be sufficient for handling all the incoming IO. Unless we didn't account for the first-page offset and the reminder coming at the last segment (which results in an extra page). The patch should fix the problem, however in this particular sg list I don't see the violation (counted 127 pages)... -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/7/2016 10:14 AM, Sagi Grimberg wrote: > >>> So this scatterlist seems perfectly fine (no gaps). The only thing I >>> can think of is that we don't have enough space reserved, but I counted >>> 127 pages in the scatterlist when iser reserves 128 (by default). >>> >>> Just in case I mis-counted, can you (or Max) try with the below >>> patch: >> >> This works fine for me, but I wonder if the miscounting is in the >> block layer? > > We communicate the "max_pages" via sg_tablesize and max_hw_sectors. > > iSER communicates (or at least supposed to) sg_tablesize=128 and > max_sectors=1024 to scsi (which sets the block queues attributes), that > should be sufficient for handling all the incoming IO. Unless we didn't > account for the first-page offset and the reminder coming at the last > segment (which results in an extra page). The patch should fix the > problem, however in this particular sg list I don't see the violation > (counted 127 pages)... I counted 129 pages. I will run some tests in our labs too. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >> We communicate the "max_pages" via sg_tablesize and max_hw_sectors. >> >> iSER communicates (or at least supposed to) sg_tablesize=128 and >> max_sectors=1024 to scsi (which sets the block queues attributes), that >> should be sufficient for handling all the incoming IO. Unless we didn't >> account for the first-page offset and the reminder coming at the last >> segment (which results in an extra page). The patch should fix the >> problem, however in this particular sg list I don't see the violation >> (counted 127 pages)... > > I counted 129 pages. > I will run some tests in our labs too. If you did then that indicates the block layer violates its obligation to respect max_segments/max_hw_sectors? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 07, 2016 at 06:16:06PM +0300, Sagi Grimberg wrote: > >>segment (which results in an extra page). The patch should fix the > >>problem, however in this particular sg list I don't see the violation > >>(counted 127 pages)... > > > >I counted 129 pages. > >I will run some tests in our labs too. > > If you did then that indicates the block layer violates > its obligation to respect max_segments/max_hw_sectors? I spent some time looking over the code and remembered an issue I fixed for NVMf (which might explain why we're not seeing it there). max_segments just means contiguous segments, not nessecarily 4k pages. For that we either need a small enough max_sectors_hw or set a max_segment_size of 4k, otherwise we might get 128 block segments, but due to lacking alignment they'll expand to 129 pages. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c index ed54b388e7ad..f2ac891c432a 100644 --- a/drivers/infiniband/ulp/iser/iser_initiator.c +++ b/drivers/infiniband/ulp/iser/iser_initiator.c @@ -250,7 +250,7 @@ int iser_alloc_rx_descriptors(struct iser_conn *iser_conn, iser_conn->min_posted_rx = iser_conn->qp_max_recv_dtos >> 2; if (device->reg_ops->alloc_reg_res(ib_conn, session->scsi_cmds_max, - iser_conn->scsi_sg_tablesize)) + iser_conn->scsi_sg_tablesize + 1)) goto create_rdma_reg_res_failed; if (iser_alloc_login_buf(iser_conn)) diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c index 40c0f4978e2f..937b97f34be1 100644 --- a/drivers/infiniband/ulp/iser/iser_verbs.c +++ b/drivers/infiniband/ulp/iser/iser_verbs.c @@ -721,7 +721,7 @@ iser_calc_scsi_params(struct iser_conn *iser_conn, sg_tablesize = DIV_ROUND_UP(max_sectors * 512, SIZE_4K); sup_sg_tablesize = min_t(unsigned, ISCSI_ISER_MAX_SG_TABLESIZE, - device->ib_device->attrs.max_fast_reg_page_list_len); + device->ib_device->attrs.max_fast_reg_page_list_len - 1);