On 09/12/2014 23:43, Roland Dreier wrote:
> I was getting ready to apply the ODP series, but then I noticed:
>
> On Tue, Nov 11, 2014 at 8:36 AM, Haggai Eran <haggaie@xxxxxxxxxxxx> wrote:
>> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
>> index 71ab83fde472..974025028790 100644
>> --- a/drivers/infiniband/core/uverbs_main.c
>> +++ b/drivers/infiniband/core/uverbs_main.c
>> @@ -122,7 +122,8 @@ static int (*uverbs_ex_cmd_table[])(struct ib_uverbs_file *file,
>> struct ib_udata *ucore,
>> struct ib_udata *uhw) = {
>> [IB_USER_VERBS_EX_CMD_CREATE_FLOW] = ib_uverbs_ex_create_flow,
>> - [IB_USER_VERBS_EX_CMD_DESTROY_FLOW] = ib_uverbs_ex_destroy_flow
>> + [IB_USER_VERBS_EX_CMD_DESTROY_FLOW] = ib_uverbs_ex_destroy_flow,
>> + [IB_USER_VERBS_EX_CMD_QUERY_DEVICE] = ib_uverbs_ex_query_device
>> };
>>
>> static void ib_uverbs_add_one(struct ib_device *device);
>
>> diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
>> index 26daf55ff76e..ed8c3d9da42c 100644
>> --- a/include/uapi/rdma/ib_user_verbs.h
>> +++ b/include/uapi/rdma/ib_user_verbs.h
>> @@ -90,8 +90,9 @@ enum {
>> };
>>
>> enum {
>> + IB_USER_VERBS_EX_CMD_QUERY_DEVICE = IB_USER_VERBS_CMD_QUERY_DEVICE,
>> IB_USER_VERBS_EX_CMD_CREATE_FLOW = IB_USER_VERBS_CMD_THRESHOLD,
>> - IB_USER_VERBS_EX_CMD_DESTROY_FLOW
>> + IB_USER_VERBS_EX_CMD_DESTROY_FLOW,
>> };
>
> And this makes no sense to me. I thought the whole point of "EX"
> commands was to add then after IB_USER_VERBS_CMD_THRESHOLD.
>
> In this case, if you make IB_USER_VERBS_EX_CMD_QUERY_DEVICE =
> IB_USER_VERBS_CMD_QUERY_DEVICE then doesn't the entry in
> uverbs_cmd_table[] for normal query device get overwritten with
> ib_uverbs_ex_query_device()??
That was my first thought as well, but a colleague pointed out that
ib_uverbs will decide whether to handle the command in the
uverbs_cmd_table[] or in the uverbs_ex_cmd_table[] based on whether
or not the IB_USER_VERBS_CMD_FLAG_EXTENDED bit in the command
opcode is on. (see ib_uverbs_write() for details, or the relevant
patch [1]). So there's actually no need for the extended verbs to
be above the threshold.
Haggai
[1] IB/core: extended command: an improved infrastructure for uverbs commands
http://www.spinics.net/lists/linux-rdma/msg17392.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
