Re: [PATCH for-next 09/10] IB/mlx4: Add timestamp_mask and hca_core_clock to query_device

[Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>]

Hi,

Le mercredi 20 mai 2015 à 17:41 +0300, Or Gerlitz a écrit :
> On 5/20/2015 3:29 AM, Jason Gunthorpe wrote:
> > On Tue, May 19, 2015 at 10:30:00PM +0300, Or Gerlitz wrote:
> > > > Are you objecting adding the clock frequency and mask to the 
> > > > qeury device verb?
> > > > why?
> > Lets see the verbs side and I'll let you know.
> 
> You mean the user series of libibverbs/libmlx4? I don't see why this 
> should be a must for the review of the kernel bits. The user-space 
> code 
> is coming up soon, sure, but we should be able to review kernel 
> patches 
> without requiring to actually see the user-space code.
> 

In some other subsystems: no userspace code, no merge.

http://blog.ffwll.ch/2015/05/gfx-kernel-upstreaming-requirements.html


> As the change-logs here explained, the clock frequency is needed for 
> applications to convert the HCA clock delta (current time - timestamp 
> on 
> WC) into nano-secs and such.The mask is needed to realize how many 
> bits
> from the 64b time-stamp are supported by the HW.
> 
> > 
> > > > If not, are objecting using the vendor specific track of the 
> > > > verb to
> > > > pass from the vendor driver to the vendor library this or that 
> > > > detail
> > > > which is needed for proper operation? why?
> > I'm uncomfortable seeing otherwise vendor-neutral calls gain vendor
> > extensions.
> 
> But this is whole purpose of the udata framework in uverbs, right? 
> for 
> each uverb command the vendor user-space library has a well defined 
> channel to communicate directly with the low level vendor driver 
> throughout the uverbs channels.
> 

Uverbs convey information between kernel and userspace drivers to
implement verbs for userspace application. I don't think it's designed
to allow vendor to add random extensions in the best way with regard to
backward/forward compability.


Anyway, please, we have to make drivers which are going to behave as
good citizens to the kernel *and* userspace. Adding a dedicated
extensions which is going to be replaced later by a generic, vendor
neutral, extension will be painful to maintain to ensure backward
compatibility.

So let's think how this timestamp extension can be made generic enough
to be future-proof (and at least present proof to address current use
cases).

Regards.

-- 
Yann Droneaud
OPTEYA



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