> > On Mon, Feb 26, 2018 at 03:22:42PM -0800, Steve Wise wrote: > > > +static int fill_res_mr_entry(struct sk_buff *msg, struct netlink_callback > *cb, > > + struct rdma_restrack_entry *res, uint32_t port) > > +{ > > + struct ib_mr *mr = container_of(res, struct ib_mr, res); > > + struct nlattr *entry_attr; > > + > > + entry_attr = nla_nest_start(msg, > RDMA_NLDEV_ATTR_RES_MR_ENTRY); > > + if (!entry_attr) > > + goto out; > > + > > + if (netlink_capable(cb->skb, CAP_NET_ADMIN)) { > > + if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_RKEY, mr- > >rkey)) > > + goto err; > > + if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_LKEY, mr- > >lkey)) > > + goto err; > > + if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_RES_IOVA, > > + mr->iova, 0)) > > ?? is '0' right here? Expecting a defined attribute constant for padding. What does the pad do exactly? I replicated other code I saw that use 0 for the pad. But I can add a NLDEV_ATTR_RES_PAD if that is the correct way to go. > > > + goto err; > > + } > > + > > + if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_RES_MRLEN, mr- > >length, 0)) > > + goto err; > > Ditto, like wise everywhere > > > + if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_PGSIZE, mr- > >page_size)) > > + goto err; > > Still not really sure what pgsize is supposed to be, I think we should > drop it?? > Let us say we register a MR that has a backing memory scattergather list of 10 64KB entries, each 64KB entry, though, is physically contiguous. The page-size for that registration, then can be 64KB even though the host page size is 4KB. So the page list for the registration with the rdma device can be 10 64K entries, each of size 64KB... > > + /* > > + * Existence of task means that it is user MR and netlink > > + * user is invited to go and read /proc/PID/comm to get name > > + * of the task file and res->task_com should be NULL. > > + */ > > + if (rdma_is_kernel_res(res)) { > > + if (nla_put_string(msg, > RDMA_NLDEV_ATTR_RES_KERN_NAME, > > + res->kern_name)) > > + goto err; > > + } else { > > + if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_PID, > > + task_pid_vnr(res->task))) > > + goto err; > > + } > > This block seems duplicated still, needs a helper I think. > I'll look into it. Thanks -- 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