On Thu, Jan 29, 2015 at 06:59:58PM +0100, Yann Droneaud wrote:
> As specified in "Extending Verbs API" presentation [1] by Tzahi Oved
> during OFA International Developer Workshop 2013, the request's
> comp_mask should describe the request data: it's describe the
> availability of extended fields in the request.
> Conversely, the response's comp_mask should describe the presence
> of extended fields in the response.
Roland: I agree with Yann, these patches need to go in, or the ODP
patches reverted.
> #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> - if (cmd.comp_mask & IB_USER_VERBS_EX_QUERY_DEVICE_ODP) {
Absolutely, a query output should never depend on the input comp_mask.
> - resp.odp_caps.general_caps = attr.odp_caps.general_caps;
> - resp.odp_caps.per_transport_caps.rc_odp_caps =
> - attr.odp_caps.per_transport_caps.rc_odp_caps;
> - resp.odp_caps.per_transport_caps.uc_odp_caps =
> - attr.odp_caps.per_transport_caps.uc_odp_caps;
> - resp.odp_caps.per_transport_caps.ud_odp_caps =
> - attr.odp_caps.per_transport_caps.ud_odp_caps;
> - resp.comp_mask |= IB_USER_VERBS_EX_QUERY_DEVICE_ODP;
> - }
> + resp.odp_caps.general_caps = attr.odp_caps.general_caps;
> + resp.odp_caps.per_transport_caps.rc_odp_caps =
> + attr.odp_caps.per_transport_caps.rc_odp_caps;
> + resp.odp_caps.per_transport_caps.uc_odp_caps =
> + attr.odp_caps.per_transport_caps.uc_odp_caps;
> + resp.odp_caps.per_transport_caps.ud_odp_caps =
> + attr.odp_caps.per_transport_caps.ud_odp_caps;
> #endif
> + resp.comp_mask |= IB_USER_VERBS_EX_QUERY_DEVICE_ODP;
Not sure about this - if 0 is a valid null answer for all the _caps
then it is fine, and the comp_mask bit should just be removed as the
size alone should be enough.
This looks wrong however:
> err = ib_copy_to_udata(ucore, &resp, sizeof(resp));
> if (err)
> return err;
> return 0;
Why isn't this returning the filled structure size to userspace? This
would seem to be very urgently wrong to me? Am I missing something?
Patch 3 probably should gain:
- return 0;
+ return resp_len;
This patch looks like an improvement to me:
Reviewed-By: Jason Gunthorpe <[email protected]>
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html