Message ID | 20201011090608.159333-1-mgurtovoy@nvidia.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [1/1] IB/isert: add module param to set sg_tablesize for IO cmd | expand |
Krishna, did this patch fix the issue you reported ? On 10/11/2020 12:06 PM, Max Gurtovoy wrote: > From: Max Gurtovoy <maxg@mellanox.com> > > Currently, iser target support max IO size of 16MiB by default. For some > adapters, allocating this amount of resources might reduce the total > number of possible connections that can be created. For those adapters, > it's preferred to reduce the max IO size to be able to create more > connections. Since there is no handshake procedure for max IO size in > iser protocol, set the default max IO size to 1MiB and add a module > parameter for enabling the option to control it for suitable adapters. > > Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com> > Signed-off-by: Max Gurtovoy <maxg@mellanox.com> > --- > drivers/infiniband/ulp/isert/ib_isert.c | 27 ++++++++++++++++++++++++- > drivers/infiniband/ulp/isert/ib_isert.h | 6 ++++++ > 2 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c > index 695f701dc43d..5a47f1bbca96 100644 > --- a/drivers/infiniband/ulp/isert/ib_isert.c > +++ b/drivers/infiniband/ulp/isert/ib_isert.c > @@ -28,6 +28,18 @@ static int isert_debug_level; > module_param_named(debug_level, isert_debug_level, int, 0644); > MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 (default:0)"); > > +static int isert_sg_tablesize_set(const char *val, > + const struct kernel_param *kp); > +static const struct kernel_param_ops sg_tablesize_ops = { > + .set = isert_sg_tablesize_set, > + .get = param_get_int, > +}; > + > +static int isert_sg_tablesize = ISCSI_ISER_SG_TABLESIZE; > +module_param_cb(sg_tablesize, &sg_tablesize_ops, &isert_sg_tablesize, 0644); > +MODULE_PARM_DESC(sg_tablesize, > + "Number of gather/scatter entries in a single scsi command, should >= 128 (default: 256, max: 4096)"); > + > static DEFINE_MUTEX(device_list_mutex); > static LIST_HEAD(device_list); > static struct workqueue_struct *isert_comp_wq; > @@ -47,6 +59,19 @@ static void isert_send_done(struct ib_cq *cq, struct ib_wc *wc); > static void isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc); > static void isert_login_send_done(struct ib_cq *cq, struct ib_wc *wc); > > +static int isert_sg_tablesize_set(const char *val, const struct kernel_param *kp) > +{ > + int n = 0, ret; > + > + ret = kstrtoint(val, 10, &n); > + if (ret != 0 || n < ISCSI_ISER_MIN_SG_TABLESIZE || > + n > ISCSI_ISER_MAX_SG_TABLESIZE) > + return -EINVAL; > + > + return param_set_int(val, kp); > +} > + > + > static inline bool > isert_prot_cmd(struct isert_conn *conn, struct se_cmd *cmd) > { > @@ -101,7 +126,7 @@ isert_create_qp(struct isert_conn *isert_conn, > attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS + 1; > attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS + 1; > factor = rdma_rw_mr_factor(device->ib_device, cma_id->port_num, > - ISCSI_ISER_MAX_SG_TABLESIZE); > + isert_sg_tablesize); > attr.cap.max_rdma_ctxs = ISCSI_DEF_XMIT_CMDS_MAX * factor; > attr.cap.max_send_sge = device->ib_device->attrs.max_send_sge; > attr.cap.max_recv_sge = 1; > diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h > index 7fee4a65e181..90ef215bf755 100644 > --- a/drivers/infiniband/ulp/isert/ib_isert.h > +++ b/drivers/infiniband/ulp/isert/ib_isert.h > @@ -65,6 +65,12 @@ > */ > #define ISER_RX_SIZE (ISCSI_DEF_MAX_RECV_SEG_LEN + 1024) > > +/* Default I/O size is 1MB */ > +#define ISCSI_ISER_SG_TABLESIZE 256 > + > +/* Minimum I/O size is 512KB */ > +#define ISCSI_ISER_MIN_SG_TABLESIZE 128 > + > /* Maximum support is 16MB I/O size */ > #define ISCSI_ISER_MAX_SG_TABLESIZE 4096 >
On 10/11/20 2:06 AM, Max Gurtovoy wrote: > From: Max Gurtovoy <maxg@mellanox.com> > > Currently, iser target support max IO size of 16MiB by default. For some > adapters, allocating this amount of resources might reduce the total > number of possible connections that can be created. For those adapters, > it's preferred to reduce the max IO size to be able to create more > connections. Since there is no handshake procedure for max IO size in > iser protocol, set the default max IO size to 1MiB and add a module > parameter for enabling the option to control it for suitable adapters. > > Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com> > Signed-off-by: Max Gurtovoy <maxg@mellanox.com> You can add a "Fixes:" tag to help it get to stable. > --- > drivers/infiniband/ulp/isert/ib_isert.c | 27 ++++++++++++++++++++++++- > drivers/infiniband/ulp/isert/ib_isert.h | 6 ++++++ > 2 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c > index 695f701dc43d..5a47f1bbca96 100644 > --- a/drivers/infiniband/ulp/isert/ib_isert.c > +++ b/drivers/infiniband/ulp/isert/ib_isert.c > @@ -28,6 +28,18 @@ static int isert_debug_level; > module_param_named(debug_level, isert_debug_level, int, 0644); > MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 (default:0)"); > > +static int isert_sg_tablesize_set(const char *val, > + const struct kernel_param *kp); > +static const struct kernel_param_ops sg_tablesize_ops = { > + .set = isert_sg_tablesize_set, > + .get = param_get_int, > +}; > + > +static int isert_sg_tablesize = ISCSI_ISER_SG_TABLESIZE; > +module_param_cb(sg_tablesize, &sg_tablesize_ops, &isert_sg_tablesize, 0644); > +MODULE_PARM_DESC(sg_tablesize, > + "Number of gather/scatter entries in a single scsi command, should >= 128 (default: 256, max: 4096)"); > + > static DEFINE_MUTEX(device_list_mutex); > static LIST_HEAD(device_list); > static struct workqueue_struct *isert_comp_wq; > @@ -47,6 +59,19 @@ static void isert_send_done(struct ib_cq *cq, struct ib_wc *wc); > static void isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc); > static void isert_login_send_done(struct ib_cq *cq, struct ib_wc *wc); > > +static int isert_sg_tablesize_set(const char *val, const struct kernel_param *kp) > +{ > + int n = 0, ret; > + > + ret = kstrtoint(val, 10, &n); > + if (ret != 0 || n < ISCSI_ISER_MIN_SG_TABLESIZE || > + n > ISCSI_ISER_MAX_SG_TABLESIZE) > + return -EINVAL; > + > + return param_set_int(val, kp); > +} > + > + > static inline bool > isert_prot_cmd(struct isert_conn *conn, struct se_cmd *cmd) > { > @@ -101,7 +126,7 @@ isert_create_qp(struct isert_conn *isert_conn, > attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS + 1; > attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS + 1; > factor = rdma_rw_mr_factor(device->ib_device, cma_id->port_num, > - ISCSI_ISER_MAX_SG_TABLESIZE); > + isert_sg_tablesize); > attr.cap.max_rdma_ctxs = ISCSI_DEF_XMIT_CMDS_MAX * factor; > attr.cap.max_send_sge = device->ib_device->attrs.max_send_sge; > attr.cap.max_recv_sge = 1; > diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h > index 7fee4a65e181..90ef215bf755 100644 > --- a/drivers/infiniband/ulp/isert/ib_isert.h > +++ b/drivers/infiniband/ulp/isert/ib_isert.h > @@ -65,6 +65,12 @@ > */ > #define ISER_RX_SIZE (ISCSI_DEF_MAX_RECV_SEG_LEN + 1024) > > +/* Default I/O size is 1MB */ > +#define ISCSI_ISER_SG_TABLESIZE 256 Maybe name it ISCSI_ISER_DEF_SG_TABLESIZE Otherwise, Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
On Tuesday, October 10/13/20, 2020 at 20:43:03 +0300, Max Gurtovoy wrote: > Krishna, > > did this patch fix the issue you reported ? With this patch applied, I don't see the reported issue anymore. Thanks for the patch! > > > On 10/11/2020 12:06 PM, Max Gurtovoy wrote: > >From: Max Gurtovoy <maxg@mellanox.com> > > > >Currently, iser target support max IO size of 16MiB by default. For some > >adapters, allocating this amount of resources might reduce the total > >number of possible connections that can be created. For those adapters, > >it's preferred to reduce the max IO size to be able to create more > >connections. Since there is no handshake procedure for max IO size in > >iser protocol, set the default max IO size to 1MiB and add a module > >parameter for enabling the option to control it for suitable adapters. > > > >Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com> > >Signed-off-by: Max Gurtovoy <maxg@mellanox.com> > >--- > > drivers/infiniband/ulp/isert/ib_isert.c | 27 ++++++++++++++++++++++++- > > drivers/infiniband/ulp/isert/ib_isert.h | 6 ++++++ > > 2 files changed, 32 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c > >index 695f701dc43d..5a47f1bbca96 100644 > >--- a/drivers/infiniband/ulp/isert/ib_isert.c > >+++ b/drivers/infiniband/ulp/isert/ib_isert.c > >@@ -28,6 +28,18 @@ static int isert_debug_level; > > module_param_named(debug_level, isert_debug_level, int, 0644); > > MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 (default:0)"); > >+static int isert_sg_tablesize_set(const char *val, > >+ const struct kernel_param *kp); > >+static const struct kernel_param_ops sg_tablesize_ops = { > >+ .set = isert_sg_tablesize_set, > >+ .get = param_get_int, > >+}; > >+ > >+static int isert_sg_tablesize = ISCSI_ISER_SG_TABLESIZE; > >+module_param_cb(sg_tablesize, &sg_tablesize_ops, &isert_sg_tablesize, 0644); > >+MODULE_PARM_DESC(sg_tablesize, > >+ "Number of gather/scatter entries in a single scsi command, should >= 128 (default: 256, max: 4096)"); > >+ > > static DEFINE_MUTEX(device_list_mutex); > > static LIST_HEAD(device_list); > > static struct workqueue_struct *isert_comp_wq; > >@@ -47,6 +59,19 @@ static void isert_send_done(struct ib_cq *cq, struct ib_wc *wc); > > static void isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc); > > static void isert_login_send_done(struct ib_cq *cq, struct ib_wc *wc); > >+static int isert_sg_tablesize_set(const char *val, const struct kernel_param *kp) > >+{ > >+ int n = 0, ret; > >+ > >+ ret = kstrtoint(val, 10, &n); > >+ if (ret != 0 || n < ISCSI_ISER_MIN_SG_TABLESIZE || > >+ n > ISCSI_ISER_MAX_SG_TABLESIZE) > >+ return -EINVAL; > >+ > >+ return param_set_int(val, kp); > >+} > >+ > >+ > > static inline bool > > isert_prot_cmd(struct isert_conn *conn, struct se_cmd *cmd) > > { > >@@ -101,7 +126,7 @@ isert_create_qp(struct isert_conn *isert_conn, > > attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS + 1; > > attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS + 1; > > factor = rdma_rw_mr_factor(device->ib_device, cma_id->port_num, > >- ISCSI_ISER_MAX_SG_TABLESIZE); > >+ isert_sg_tablesize); > > attr.cap.max_rdma_ctxs = ISCSI_DEF_XMIT_CMDS_MAX * factor; > > attr.cap.max_send_sge = device->ib_device->attrs.max_send_sge; > > attr.cap.max_recv_sge = 1; > >diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h > >index 7fee4a65e181..90ef215bf755 100644 > >--- a/drivers/infiniband/ulp/isert/ib_isert.h > >+++ b/drivers/infiniband/ulp/isert/ib_isert.h > >@@ -65,6 +65,12 @@ > > */ > > #define ISER_RX_SIZE (ISCSI_DEF_MAX_RECV_SEG_LEN + 1024) > >+/* Default I/O size is 1MB */ > >+#define ISCSI_ISER_SG_TABLESIZE 256 > >+ > >+/* Minimum I/O size is 512KB */ > >+#define ISCSI_ISER_MIN_SG_TABLESIZE 128 > >+ > > /* Maximum support is 16MB I/O size */ > > #define ISCSI_ISER_MAX_SG_TABLESIZE 4096
On 10/14/2020 1:21 AM, Sagi Grimberg wrote: > > > On 10/11/20 2:06 AM, Max Gurtovoy wrote: >> From: Max Gurtovoy <maxg@mellanox.com> >> >> Currently, iser target support max IO size of 16MiB by default. For some >> adapters, allocating this amount of resources might reduce the total >> number of possible connections that can be created. For those adapters, >> it's preferred to reduce the max IO size to be able to create more >> connections. Since there is no handshake procedure for max IO size in >> iser protocol, set the default max IO size to 1MiB and add a module >> parameter for enabling the option to control it for suitable adapters. >> >> Reported-by: Krishnamraju Eraparaju <krishna2@chelsio.com> >> Signed-off-by: Max Gurtovoy <maxg@mellanox.com> > > You can add a "Fixes:" tag to help it get to stable. But the previous commit is not buggy. It reveals some resource lacking in some HCAs. I can add it if needed. > >> --- >> drivers/infiniband/ulp/isert/ib_isert.c | 27 ++++++++++++++++++++++++- >> drivers/infiniband/ulp/isert/ib_isert.h | 6 ++++++ >> 2 files changed, 32 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c >> b/drivers/infiniband/ulp/isert/ib_isert.c >> index 695f701dc43d..5a47f1bbca96 100644 >> --- a/drivers/infiniband/ulp/isert/ib_isert.c >> +++ b/drivers/infiniband/ulp/isert/ib_isert.c >> @@ -28,6 +28,18 @@ static int isert_debug_level; >> module_param_named(debug_level, isert_debug_level, int, 0644); >> MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 >> (default:0)"); >> +static int isert_sg_tablesize_set(const char *val, >> + const struct kernel_param *kp); >> +static const struct kernel_param_ops sg_tablesize_ops = { >> + .set = isert_sg_tablesize_set, >> + .get = param_get_int, >> +}; >> + >> +static int isert_sg_tablesize = ISCSI_ISER_SG_TABLESIZE; >> +module_param_cb(sg_tablesize, &sg_tablesize_ops, >> &isert_sg_tablesize, 0644); >> +MODULE_PARM_DESC(sg_tablesize, >> + "Number of gather/scatter entries in a single scsi command, >> should >= 128 (default: 256, max: 4096)"); >> + >> static DEFINE_MUTEX(device_list_mutex); >> static LIST_HEAD(device_list); >> static struct workqueue_struct *isert_comp_wq; >> @@ -47,6 +59,19 @@ static void isert_send_done(struct ib_cq *cq, >> struct ib_wc *wc); >> static void isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc); >> static void isert_login_send_done(struct ib_cq *cq, struct ib_wc *wc); >> +static int isert_sg_tablesize_set(const char *val, const struct >> kernel_param *kp) >> +{ >> + int n = 0, ret; >> + >> + ret = kstrtoint(val, 10, &n); >> + if (ret != 0 || n < ISCSI_ISER_MIN_SG_TABLESIZE || >> + n > ISCSI_ISER_MAX_SG_TABLESIZE) >> + return -EINVAL; >> + >> + return param_set_int(val, kp); >> +} >> + >> + >> static inline bool >> isert_prot_cmd(struct isert_conn *conn, struct se_cmd *cmd) >> { >> @@ -101,7 +126,7 @@ isert_create_qp(struct isert_conn *isert_conn, >> attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS + 1; >> attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS + 1; >> factor = rdma_rw_mr_factor(device->ib_device, cma_id->port_num, >> - ISCSI_ISER_MAX_SG_TABLESIZE); >> + isert_sg_tablesize); >> attr.cap.max_rdma_ctxs = ISCSI_DEF_XMIT_CMDS_MAX * factor; >> attr.cap.max_send_sge = device->ib_device->attrs.max_send_sge; >> attr.cap.max_recv_sge = 1; >> diff --git a/drivers/infiniband/ulp/isert/ib_isert.h >> b/drivers/infiniband/ulp/isert/ib_isert.h >> index 7fee4a65e181..90ef215bf755 100644 >> --- a/drivers/infiniband/ulp/isert/ib_isert.h >> +++ b/drivers/infiniband/ulp/isert/ib_isert.h >> @@ -65,6 +65,12 @@ >> */ >> #define ISER_RX_SIZE (ISCSI_DEF_MAX_RECV_SEG_LEN + 1024) >> +/* Default I/O size is 1MB */ >> +#define ISCSI_ISER_SG_TABLESIZE 256 > > Maybe name it ISCSI_ISER_DEF_SG_TABLESIZE ok > > Otherwise, > > Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c index 695f701dc43d..5a47f1bbca96 100644 --- a/drivers/infiniband/ulp/isert/ib_isert.c +++ b/drivers/infiniband/ulp/isert/ib_isert.c @@ -28,6 +28,18 @@ static int isert_debug_level; module_param_named(debug_level, isert_debug_level, int, 0644); MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 (default:0)"); +static int isert_sg_tablesize_set(const char *val, + const struct kernel_param *kp); +static const struct kernel_param_ops sg_tablesize_ops = { + .set = isert_sg_tablesize_set, + .get = param_get_int, +}; + +static int isert_sg_tablesize = ISCSI_ISER_SG_TABLESIZE; +module_param_cb(sg_tablesize, &sg_tablesize_ops, &isert_sg_tablesize, 0644); +MODULE_PARM_DESC(sg_tablesize, + "Number of gather/scatter entries in a single scsi command, should >= 128 (default: 256, max: 4096)"); + static DEFINE_MUTEX(device_list_mutex); static LIST_HEAD(device_list); static struct workqueue_struct *isert_comp_wq; @@ -47,6 +59,19 @@ static void isert_send_done(struct ib_cq *cq, struct ib_wc *wc); static void isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc); static void isert_login_send_done(struct ib_cq *cq, struct ib_wc *wc); +static int isert_sg_tablesize_set(const char *val, const struct kernel_param *kp) +{ + int n = 0, ret; + + ret = kstrtoint(val, 10, &n); + if (ret != 0 || n < ISCSI_ISER_MIN_SG_TABLESIZE || + n > ISCSI_ISER_MAX_SG_TABLESIZE) + return -EINVAL; + + return param_set_int(val, kp); +} + + static inline bool isert_prot_cmd(struct isert_conn *conn, struct se_cmd *cmd) { @@ -101,7 +126,7 @@ isert_create_qp(struct isert_conn *isert_conn, attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS + 1; attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS + 1; factor = rdma_rw_mr_factor(device->ib_device, cma_id->port_num, - ISCSI_ISER_MAX_SG_TABLESIZE); + isert_sg_tablesize); attr.cap.max_rdma_ctxs = ISCSI_DEF_XMIT_CMDS_MAX * factor; attr.cap.max_send_sge = device->ib_device->attrs.max_send_sge; attr.cap.max_recv_sge = 1; diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h index 7fee4a65e181..90ef215bf755 100644 --- a/drivers/infiniband/ulp/isert/ib_isert.h +++ b/drivers/infiniband/ulp/isert/ib_isert.h @@ -65,6 +65,12 @@ */ #define ISER_RX_SIZE (ISCSI_DEF_MAX_RECV_SEG_LEN + 1024) +/* Default I/O size is 1MB */ +#define ISCSI_ISER_SG_TABLESIZE 256 + +/* Minimum I/O size is 512KB */ +#define ISCSI_ISER_MIN_SG_TABLESIZE 128 + /* Maximum support is 16MB I/O size */ #define ISCSI_ISER_MAX_SG_TABLESIZE 4096