[ofa-general] [PATCH 3/3 v2] ib/uverbs: add support for create_qp_expanded in uverbs

[Ron Livne]

> You're still not using a separate cmd structure for ib_uverbs_create_qp_expanded.
> It is NOT a good idea to use the 8-bit reserved field for ib_uverbs_create_qp_expanded().
> You should use a different struct (as I indicated in my previous mail).

Sorry, I couldn't find the email you were talking about. can you
please resend it to me?
Why isn't it good to use the reserved field? Will it not just create
code duplication for creating a regular qp, and qp_expanded?

Ron


On Tue, Jul 22, 2008 at 9:59 AM, Jack Morgenstein
<jackm at dev.mellanox.co.il> wrote:
> General comment:
> Please run checkpatch.pl -- there are lots of formatting errors here.
> See additional comments below.
>
> - Jack
>
> On Monday 21 July 2008 23:27, ronli at voltaire.com wrote:
>> -ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
>> -                         const char __user *buf, int in_len,
>> -                         int out_len)
>> +static ssize_t ib_uverbs_create_qp_common(struct ib_uverbs_file *file,
>> +                                     const char __user *buf, int in_len,
>> +                                     int out_len, int expanded)
>>  {
>
>> -     struct ib_uverbs_create_qp      cmd;
>> +     struct ib_uverbs_create_qp cmd;
> unneeded modification
>
>>       struct ib_uverbs_create_qp_resp resp;
>>       struct ib_udata                 udata;
>>       struct ib_uqp_object           *obj;
>> @@ -1078,7 +1078,6 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
>>               goto err_put;
>>       }
>>
>> -     attr.create_flags  = 0;
>>       attr.event_handler = ib_uverbs_qp_event_handler;
>>       attr.qp_context    = file;
>>       attr.send_cq       = scq;
>> @@ -1087,7 +1086,7 @@ ssize_t ib_uverbs_create_qp(struct ib_uverbs_file *file,
>>       attr.sq_sig_type   = cmd.sq_sig_all ? IB_SIGNAL_ALL_WR : IB_SIGNAL_REQ_WR;
>>       attr.qp_type       = cmd.qp_type;
>>       attr.xrc_domain    = xrcd;
>> -     attr.create_flags  = 0;
>> +     attr.create_flags  = expanded ? cmd.reserved : 0;
> You're still not using a separate cmd structure for ib_uverbs_create_qp_expanded.
> It is NOT a good idea to use the 8-bit reserved field for ib_uverbs_create_qp_expanded().
> You should use a different struct (as I indicated in my previous mail).
>
> The common part is more difficult to isolate, since you have different cmd structs, but
> it is possible.
>
>>
>> @@ -722,6 +726,7 @@ struct ib_qp *mlx4_ib_create_qp(struct ib_pd *pd,
>>                       return ERR_PTR(-ENOMEM);
>>
>>               err = create_qp_common(dev, pd, init_attr, udata, 0, qp);
>> +
> unneeded change
>>               if (err) {
>>                       kfree(qp);
>>                       return ERR_PTR(err);
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>