On Mon, Sep 23, 2019 at 10:11:06AM -0700, Bart Van Assche wrote:
> On 9/22/19 11:29 PM, Honggang LI wrote:
> > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> > index b829dab0df77..b3bf5d552de9 100644
> > --- a/drivers/infiniband/ulp/srp/ib_srp.c
> > +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> > @@ -139,7 +139,7 @@ MODULE_PARM_DESC(use_imm_data,
> > static unsigned int srp_max_imm_data = 8 * 1024;
> > module_param_named(max_imm_data, srp_max_imm_data, uint, 0644);
> > -MODULE_PARM_DESC(max_imm_data, "Maximum immediate data size.");
> > +MODULE_PARM_DESC(max_imm_data, "Maximum immediate data size if max_it_iu_size has not been specified.");
> > static unsigned ch_count;
> > module_param(ch_count, uint, 0444);
> > @@ -1362,15 +1362,23 @@ static void srp_terminate_io(struct srp_rport *rport)
> > }
> > /* Calculate maximum initiator to target information unit length. */
> > -static uint32_t srp_max_it_iu_len(int cmd_sg_cnt, bool use_imm_data)
> > +static uint32_t srp_max_it_iu_len(int cmd_sg_cnt, bool use_imm_data,
> > + uint32_t max_it_iu_size)
> > {
> > uint32_t max_iu_len = sizeof(struct srp_cmd) + SRP_MAX_ADD_CDB_LEN +
> > sizeof(struct srp_indirect_buf) +
> > cmd_sg_cnt * sizeof(struct srp_direct_buf);
> > - if (use_imm_data)
> > - max_iu_len = max(max_iu_len, SRP_IMM_DATA_OFFSET +
> > - srp_max_imm_data);
> > + if (use_imm_data) {
> > + if (max_it_iu_size == 0) {
> > + max_iu_len = max(max_iu_len,
> > + SRP_IMM_DATA_OFFSET + srp_max_imm_data);
> > + } else {
> > + max_iu_len = max_it_iu_size;
> > + }
> > + }
> > +
> > + pr_debug("max_iu_len = %d\n", max_iu_len);
> > return max_iu_len;
> > }
>
> Thinking further about this, this removes the ability for users to limit
> immediate data to a certain number of bytes. I think that's a step back. How
> about not modifying the description of max_imm_data and to use the following
> approach in srp_max_it_iu_len() for calculating max_iu_len?
Sounds good. I will send new patch to address this.
thanks
>
> uint32_t max_iu_len = sizeof(struct srp_cmd) + SRP_MAX_ADD_CDB_LEN +
> sizeof(struct srp_indirect_buf) +
> cmd_sg_cnt * sizeof(struct srp_direct_buf);
> if (use_imm_data)
> max_iu_len = max(max_iu_len, SRP_IMM_DATA_OFFSET +
> srp_max_imm_data);
> if (max_it_iu_size)
> max_iu_len = min(max_iu_len, max_it_iu_size);
>
> Thanks,
>
> Bart.
