On 26-Feb-19 23:43, Steve Wise wrote:
>> +int efa_query_port(struct ib_device *ibdev, u8 port,
>> + struct ib_port_attr *props)
>> +{
>> + struct efa_dev *dev = to_edev(ibdev);
>> +
>> + memset(props, 0, sizeof(*props));
>> +
>> + props->lid = 0;
>> + props->lmc = 1;
>> + props->sm_lid = 0;
>> + props->sm_sl = 0;
>> +
>> + props->state = IB_PORT_ACTIVE;
>> + props->phys_state = 5;
>> + props->port_cap_flags = 0;
>> + props->gid_tbl_len = 1;
>> + props->pkey_tbl_len = 1;
>> + props->bad_pkey_cntr = 0;
>> + props->qkey_viol_cntr = 0;
>> + props->active_speed = IB_SPEED_EDR;
>> + props->active_width = IB_WIDTH_4X;
>> + props->max_mtu = ib_mtu_int_to_enum(dev->mtu);
>> + props->active_mtu = ib_mtu_int_to_enum(dev->mtu);
>> + props->max_msg_sz = dev->mtu;
>> + props->max_vl_num = 1;
>> +
>
>
> Since you memset() props to all zeros, should you bother with
> initializing the zero fields?
Will remove.
>
>
>> + return 0;
>> +}
>> +
>> +static int efa_qp_validate_cap(struct efa_dev *dev,
>> + struct ib_qp_init_attr *init_attr)
>> +{
>> + if (init_attr->cap.max_send_wr > dev->dev_attr.max_sq_depth) {
>> + efa_err(&dev->ibdev.dev,
>> + "qp: requested send wr[%u] exceeds the max[%u]\n",
>> + init_attr->cap.max_send_wr,
>> + dev->dev_attr.max_sq_depth);
>> + return -EINVAL;
>> + }
>> + if (init_attr->cap.max_recv_wr > dev->dev_attr.max_rq_depth) {
>> + efa_err(&dev->ibdev.dev,
>> + "qp: requested receive wr[%u] exceeds the max[%u]\n",
>> + init_attr->cap.max_recv_wr,
>> + dev->dev_attr.max_rq_depth);
>> + return -EINVAL;
>> + }
>> + if (init_attr->cap.max_send_sge > dev->dev_attr.max_sq_sge) {
>> + efa_err(&dev->ibdev.dev,
>> + "qp: requested sge send[%u] exceeds the max[%u]\n",
>> + init_attr->cap.max_send_sge, dev->dev_attr.max_sq_sge);
>> + return -EINVAL;
>> + }
>> + if (init_attr->cap.max_recv_sge > dev->dev_attr.max_rq_sge) {
>> + efa_err(&dev->ibdev.dev,
>> + "qp: requested sge recv[%u] exceeds the max[%u]\n",
>> + init_attr->cap.max_recv_sge, dev->dev_attr.max_rq_sge);
>> + return -EINVAL;
>> + }
>> + if (init_attr->cap.max_inline_data > dev->dev_attr.inline_buf_size) {
>> + efa_err(&dev->ibdev.dev,
>> + "requested inline data[%u] exceeds the max[%u]\n",
>> + init_attr->cap.max_inline_data,
>> + dev->dev_attr.inline_buf_size);
>> + return -EINVAL;
>> + }
>> +
>
>
> Should all these efa_err() calls really be efa_dbg()s? That's a lot of
> log polluting for user errors.
Most users don't really enable debug and we want them to have an indication of
what happened. is efa_warn() better?
>
>
>> + return 0;
>> +}
>> +
>> +static struct scatterlist *efa_vmalloc_buf_to_sg(u64 *buf, int page_cnt)
>> +{
>> + struct scatterlist *sglist;
>> + struct page *pg;
>> + int i;
>> +
>> + sglist = kcalloc(page_cnt, sizeof(*sglist), GFP_KERNEL);
>> + if (!sglist)
>> + return NULL;
>> + sg_init_table(sglist, page_cnt);
>> + for (i = 0; i < page_cnt; i++) {
>> + pg = vmalloc_to_page(buf);
>> + if (!pg)
>> + goto err;
>> + WARN_ON_ONCE(PageHighMem(pg));
>
> Is this WARN_ON_ONCE() really an error that needs to be handled?
AFAIK, there is no way we can actually get a higemem page here.
The WARN is here from early dev days, it should probably be removed.
>
>
>> + sg_set_page(&sglist[i], pg, EFA_PAGE_SIZE, 0);
>> + buf = (u64 *)((u8 *)buf + EFA_PAGE_SIZE);
>> + }
>> + return sglist;
>> +
>> +err:
>> + kfree(sglist);
>> + return NULL;
>> +}
>> +
>
>
> Reviewed-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
Thanks Steve!
