| Message ID | 20200522213341.16341-2-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | SRP initiator and target patches for kernel v5.8 | expand |
On Fri, May 22, 2020 at 02:33:38PM -0700, Bart Van Assche wrote: > Increase the flexibility of the SRP initiator driver by making the channel > count configurable per target instead of only providing a kernel module > parameter for configuring the channel count. > > Cc: Laurence Oberman <loberman@redhat.com> > Cc: Kamal Heib <kamalheib1@gmail.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c > index 00b4f88b113e..d686c39710c0 100644 > --- a/drivers/infiniband/ulp/srp/ib_srp.c > +++ b/drivers/infiniband/ulp/srp/ib_srp.c > @@ -3424,6 +3424,7 @@ enum { > SRP_OPT_IP_DEST = 1 << 16, > SRP_OPT_TARGET_CAN_QUEUE= 1 << 17, > SRP_OPT_MAX_IT_IU_SIZE = 1 << 18, > + SRP_OPT_CH_COUNT = 1 << 19, > }; > > static unsigned int srp_opt_mandatory[] = { > @@ -3457,6 +3458,7 @@ static const match_table_t srp_opt_tokens = { > { SRP_OPT_IP_SRC, "src=%s" }, > { SRP_OPT_IP_DEST, "dest=%s" }, > { SRP_OPT_MAX_IT_IU_SIZE, "max_it_iu_size=%d" }, > + { SRP_OPT_CH_COUNT, "ch_count=%d", }, Why did you use %d and not %u? Thanks
On 2020-05-23 06:55, Leon Romanovsky wrote: > On Fri, May 22, 2020 at 02:33:38PM -0700, Bart Van Assche wrote: >> Increase the flexibility of the SRP initiator driver by making the channel >> count configurable per target instead of only providing a kernel module >> parameter for configuring the channel count. >> >> Cc: Laurence Oberman <loberman@redhat.com> >> Cc: Kamal Heib <kamalheib1@gmail.com> >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> >> --- >> drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++++++++----- >> 1 file changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c >> index 00b4f88b113e..d686c39710c0 100644 >> --- a/drivers/infiniband/ulp/srp/ib_srp.c >> +++ b/drivers/infiniband/ulp/srp/ib_srp.c >> @@ -3424,6 +3424,7 @@ enum { >> SRP_OPT_IP_DEST = 1 << 16, >> SRP_OPT_TARGET_CAN_QUEUE= 1 << 17, >> SRP_OPT_MAX_IT_IU_SIZE = 1 << 18, >> + SRP_OPT_CH_COUNT = 1 << 19, >> }; >> >> static unsigned int srp_opt_mandatory[] = { >> @@ -3457,6 +3458,7 @@ static const match_table_t srp_opt_tokens = { >> { SRP_OPT_IP_SRC, "src=%s" }, >> { SRP_OPT_IP_DEST, "dest=%s" }, >> { SRP_OPT_MAX_IT_IU_SIZE, "max_it_iu_size=%d" }, >> + { SRP_OPT_CH_COUNT, "ch_count=%d", }, > > Why did you use %d and not %u? Hi Leon, There is more kernel code that uses %d for unsigned integers. Anyway, if someone cares enough I can change %d into %u. Bart.
On Sat, May 23, 2020 at 08:22:19AM -0700, Bart Van Assche wrote: > On 2020-05-23 06:55, Leon Romanovsky wrote: > > On Fri, May 22, 2020 at 02:33:38PM -0700, Bart Van Assche wrote: > >> Increase the flexibility of the SRP initiator driver by making the channel > >> count configurable per target instead of only providing a kernel module > >> parameter for configuring the channel count. > >> > >> Cc: Laurence Oberman <loberman@redhat.com> > >> Cc: Kamal Heib <kamalheib1@gmail.com> > >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> > >> --- > >> drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++++++++----- > >> 1 file changed, 16 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c > >> index 00b4f88b113e..d686c39710c0 100644 > >> --- a/drivers/infiniband/ulp/srp/ib_srp.c > >> +++ b/drivers/infiniband/ulp/srp/ib_srp.c > >> @@ -3424,6 +3424,7 @@ enum { > >> SRP_OPT_IP_DEST = 1 << 16, > >> SRP_OPT_TARGET_CAN_QUEUE= 1 << 17, > >> SRP_OPT_MAX_IT_IU_SIZE = 1 << 18, > >> + SRP_OPT_CH_COUNT = 1 << 19, > >> }; > >> > >> static unsigned int srp_opt_mandatory[] = { > >> @@ -3457,6 +3458,7 @@ static const match_table_t srp_opt_tokens = { > >> { SRP_OPT_IP_SRC, "src=%s" }, > >> { SRP_OPT_IP_DEST, "dest=%s" }, > >> { SRP_OPT_MAX_IT_IU_SIZE, "max_it_iu_size=%d" }, > >> + { SRP_OPT_CH_COUNT, "ch_count=%d", }, > > > > Why did you use %d and not %u? > > Hi Leon, > > There is more kernel code that uses %d for unsigned integers. Anyway, if > someone cares enough I can change %d into %u. I don't have strong opinion about that. Thanks > > Bart.
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 00b4f88b113e..d686c39710c0 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -3424,6 +3424,7 @@ enum { SRP_OPT_IP_DEST = 1 << 16, SRP_OPT_TARGET_CAN_QUEUE= 1 << 17, SRP_OPT_MAX_IT_IU_SIZE = 1 << 18, + SRP_OPT_CH_COUNT = 1 << 19, }; static unsigned int srp_opt_mandatory[] = { @@ -3457,6 +3458,7 @@ static const match_table_t srp_opt_tokens = { { SRP_OPT_IP_SRC, "src=%s" }, { SRP_OPT_IP_DEST, "dest=%s" }, { SRP_OPT_MAX_IT_IU_SIZE, "max_it_iu_size=%d" }, + { SRP_OPT_CH_COUNT, "ch_count=%d", }, { SRP_OPT_ERR, NULL } }; @@ -3758,6 +3760,14 @@ static int srp_parse_options(struct net *net, const char *buf, target->max_it_iu_size = token; break; + case SRP_OPT_CH_COUNT: + if (match_int(args, &token) || token < 1) { + pr_warn("bad channel count %s\n", p); + goto out; + } + target->ch_count = token; + break; + default: pr_warn("unknown parameter or missing value '%s' in target creation request\n", p); @@ -3921,11 +3931,12 @@ static ssize_t srp_create_target(struct device *dev, goto out; ret = -ENOMEM; - target->ch_count = max_t(unsigned, num_online_nodes(), - min(ch_count ? : - min(4 * num_online_nodes(), - ibdev->num_comp_vectors), - num_online_cpus())); + if (target->ch_count == 0) + target->ch_count = max_t(unsigned, num_online_nodes(), + min(ch_count ? : + min(4 * num_online_nodes(), + ibdev->num_comp_vectors), + num_online_cpus())); target->ch = kcalloc(target->ch_count, sizeof(*target->ch), GFP_KERNEL); if (!target->ch)
Increase the flexibility of the SRP initiator driver by making the channel count configurable per target instead of only providing a kernel module parameter for configuring the channel count. Cc: Laurence Oberman <loberman@redhat.com> Cc: Kamal Heib <kamalheib1@gmail.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/infiniband/ulp/srp/ib_srp.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)