Re: [PATCH rdma-next 1/6] IB/core: Save the device attributes on the device structure

[Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]

On 12/17/2015 7:41 PM, Jason Gunthorpe wrote:
> On Thu, Dec 17, 2015 at 03:44:19PM +0200, Sagi Grimberg wrote:
>>> +	ret = ib_query_device(device, &device->attrs);
>>> +	if (ret) {
>>> +		printk(KERN_WARNING "Couldn't query the device attributes\n");
>>> +		goto out;
>>> +	}
>>> +
>> I thought we're all for removing the call altogether aren't we?
>>
>> I'd say just call device->query_device() instead.
> Christoph's patch even got rid of device->query_device(),

Wrong.

UDATA is a mechanism added by the founders of the RDMA stack to allow 
for LL HW
driver pass data to LL HW user space library piggy backed on uverbs 
commands.

For example, mlx4 uses that to path details of the HW clock which are 
needed for libmlx4.

Christoph noted that and didn't remove the query_device entry from mlx4, he
also madethis somehow inelegant change to uverbs_cmd.c:

@@ -3624,26 +3607,29 @@ int ib_uverbs_ex_query_device(struct 
ib_uverbs_file *file,
         if (ucore->outlen < resp.response_length)
                 return -ENOSPC;

-       memset(&attr, 0, sizeof(attr));
-
-       err = ib_dev->query_device(ib_dev, &attr, uhw);
-       if (err)
-               return err;
+       if (ib_dev->query_device) {
+               int err = ib_dev->query_device(ib_dev, uhw);
+               if (err)
+                       return err;
+       } else {
+               if (uhw->inlen || uhw->outlen)
+                       return -EINVAL;
+       }


> which, IHMO, I prefer to see over this.

It would be wrong to disallow LL drivers to pass data back and forth to/from
their user space sister libraries, this is part of a 10Y old kernel UAPI.

> It re-enforces that these values are constants and drivers cannot change them on the fly.

yes, we should be looking on how to constify things, but incrementally.
I don't see why block this cleanup now as of this point.

Or.

--
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