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

Yann Droneaud
2015-05-20T15:11:17Z

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