Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 08 Sep 2014 16:25:40 +0200
From:      =?windows-1252?Q?Jean-S=E9bastien_P=E9dron?= <dumbbell@FreeBSD.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        freebsd-arch@FreeBSD.org
Subject:   Re: Linux kernel API wrapper: using OFED's one in other drivers
Message-ID:  <540DBC64.7050302@FreeBSD.org>
In-Reply-To: <20140908103908.GZ2737@kib.kiev.ua>
References:  <540D7D91.9000104@FreeBSD.org> <20140908103908.GZ2737@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--p5mxqV1VW8IGhjcTf095IU0DxIa5mM0qW
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable

On 08.09.2014 12:39, Konstantin Belousov wrote:
> My main objections to use OFED wrappers for drm2 are:
>=20
> 1. It tightly binds different drivers with non-coordinating maintainers=

>    to the version of the Linux KPI.  The Linux drivers interface is not=

>    known for its stability, and constant drift there in both formal
>    spelling of interfaces and in semantic requires to have all drivers
>    using the wrapper to be at the same upstream version.
>=20
>    This objection is not specific to drm code, but so far we only speek=

>    about infiniband and drm.

A small note before going on: cxgb(4) and cxgbe(4) use OFED's wrapper too=
=2E

> 2. This cause is probably very drm-specific.  Comparing with infiniband=
,
>    drm integration with very core of the FreeBSD kernel is quite
>    significant.  In particular, its coupling with the VM subsystem
>    has no prior examples among the drivers, even if you consider the
>    removed zero-copy sockets code or any driver providing dev_mmap
>    cdevsw method. Even NVidia blob did not got that level of integratio=
n
>    with the VM, but probably would benefit from it.  The only remotely
>    close example is vbox, but I usually avoid reading that code.
>=20
>    I mean things like managed fictitious pages, new device pager
>    page fault handler, or CPU cache management.  All of this is used
>    both by i195/GEM and TTM.
>=20
>    With the use of the listed facilities, ported driver code 'feels' li=
ke
>    FreeBSD code, not Linux code.  Mix of the Linux KPI-compatible wrapp=
ers
>    for shallow facilities, and native interfaces for anything more
>    involved, makes the code which is hard to understand for either Free=
BSD
>    or Linux developers.  This was the reason why I selected the
>    'no wrappers' approach for initial port of GEM, and continue to
>    prefer it now.

You're right the Linux interface instability is even a documented
feature [1]. And yes, I was only considering facilities such as data
structures (list, idr), taskqueues/workqueues, malloc/kmalloc or some
printf()/KASSERT() macros. I don't want to eliminate the diff with
Linux, only reduce it. The integration with the VM is out of the scope
of this proposal and locking primitives are to be considered carefully
if we want to wrap them (ie. out of the scope for now too).

The facilities I mention are a small piece of code. I believe they are
far more easy to maintain than replacing every single calls to malloc()
and friends and then handling conflicts when trying to import new changes=
=2E

To address the instability problem, we could have a directory for each
used versions and a directory for the common files. For instance:
    linux/common/linux/types.h
    linux/3.8/linux/idr.h
    linux/3.8/linux/idr.c
(the hierarchy and files are fake)

And in idr.c, we could have idr_for_each_38() (hidden behind a macro in
idr.h) if the API changed since eg. Linux 3.4. A consumer of this
wrapper would need to include both directories:
    -I.../linux/3.8 -I.../linux/common

I can't comment on the frequency of disruptive changes in the Linux API.
By quickly looking at the history for eg. idr.[ch], it's hard to tell.
If you feel that it moves too fast, then I don't mind having a
DRM-specific wrapper.

About the difficulty to read the code and the absence of "feels like
home" impression, it's true that the reader would need to know both
kernels. But a FreeBSD contributor who wants to work on this component
already has to read Linux source anyway, because today, the code only
flows in one direction. I find this problem acceptable compared to the
benefit.

My main point is that we already struggle with the maintenance of the
DRM subsystem and two drivers. I believe a wrapper would remove a lot
from that "pain".

[1] https://www.kernel.org/doc/Documentation/stable_api_nonsense.txt

--=20
Jean-S=E9bastien P=E9dron


--p5mxqV1VW8IGhjcTf095IU0DxIa5mM0qW
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQJ8BAEBCgBmBQJUDbxxXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXQ2NzA4N0ZEMUFFQUUwRTEyREJDNkE2RjAz
OUU5OTc2MUE1RkQ5NENDAAoJEDnpl2Gl/ZTMB4EP/i42W7pYSwHBlV6tBeyXX6qY
4BPVcP5Vve/+KTErEWavNud4JCeVnLHqO7+hpMjyesab3p6qju1TZC7KjkEoyys7
O1OjJX07057HvhD1ihX9WN/JbNH1HDjVQWtGYOfLetu31gPt53h8uKpBTY5BwjXu
BQhPutpwhOtmOLmL9+8y8JG0ccAhilo+wgbmK495mHSX28BTiace8BgnYLC0sGmh
N9AakjYAnGEmCA3BGUkjnHWiZLjMKELMh8O3YtINS3CXGEYGebizCt8SIwV8Hgeu
kb+2nvXcb9IDxmDUZ7/S7TTQsjdK/BktMYVllnFFoRS5NKiBs5K54b3cdhkjI3FA
fy2a49dz8jRu2LVKh3ye3kLXxw40F6gZTTyUK7FD7uSDlvnMVbyuC2auuSzYv3wT
4tdtyUeBNgs2db2cjdjVoy2YgagVnucUyDmnZH5yF3/UvwiVAwgRnLOINDY+8cAZ
OWX9ZvDZmWZ/TDba0kT7v927COwqQy81pfq11xP03eil0SZqe8KG3zRC+5Se9XlS
DGDr7Lg/ia3/mg56UL2i62tqb3YDvcWxxcvAqST84u/FBUEYLj7iwdSC34epa96E
NOfO1vF/1p5luhEamFJeYjP0hZtYYPil0KLYnSp82ZGuWLFc7Fh3j14iFk6nD7JO
arfUNZo16VyApJe1QFln
=MfNd
-----END PGP SIGNATURE-----

--p5mxqV1VW8IGhjcTf095IU0DxIa5mM0qW--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?540DBC64.7050302>