Re: [PATCH V2 libibverbs 2/7] Add member functions to poll an extended CQ

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Thu, May 19, 2016 at 02:51:37PM +0300, Yishai Hadas wrote:
> +	int (*start_poll_ex)(struct ibv_cq_ex *current,
> +			     struct ibv_poll_cq_ex_attr *attr);
> +	int (*next_poll_ex)(struct ibv_cq_ex *current);
> +	void (*end_poll_ex)(struct ibv_cq_ex *current);

Probably don't need the _ex here..

> +	uint64_t (*read_wr_id)(struct ibv_cq_ex *current);
> +	enum ibv_wc_status (*read_status)(struct ibv_cq_ex *current);
> +	enum ibv_wc_opcode (*read_opcode)(struct ibv_cq_ex *current);

opcode, status and wr_id are always available in every WR, so I think
a faster way to return them should be devised. Perahps have the driver
write them to ibv_cq_ex and have the inline functions read the struct
directly?

How does this work multi-threaded? Is the locking done in
start_poll.end_poll?

> +	uint32_t (*read_vendor_err)(struct ibv_cq_ex *current);
> +	uint32_t (*read_byte_len)(struct ibv_cq_ex *current);
> +	uint32_t (*read_imm_data)(struct ibv_cq_ex *current);
> +	uint32_t (*read_qp_num)(struct ibv_cq_ex *current);
> +	uint32_t (*read_src_qp)(struct ibv_cq_ex *current);
> +	int (*read_wc_flags)(struct ibv_cq_ex *current);
> +	uint16_t (*read_pkey_index)(struct ibv_cq_ex *current);
> +	uint16_t (*read_slid)(struct ibv_cq_ex *current);
> +	uint8_t (*read_sl)(struct ibv_cq_ex *current);
> +	uint8_t (*read_dlid_path_bits)(struct ibv_cq_ex *current);

Maybe some of these should be grouped into a struct? What kind of
performance hit did you measure on these calls? Reading all the ud
parameters is not very expensive...

read_ud_info ?

What does the driver look like? I'm assuming it turned out as short
and jump free as I outlined?

Finally, these extended APIs really need to be always available - so
the core code should always provide a compatability wrapper for
providers that do not implement this API. The compat wrapper should
use the existing provider wc interface.

We should not have stuff like in your rc_pingpong example where
totally different code paths are required on something like wc
processing.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html