Linux kernel API wrapper: using OFED's one in other drivers

[Jean-Sébastien Pédron]

On 08.09.2014 12:39, Konstantin Belousov wrote:
> My main objections to use OFED wrappers for drm2 are:
> 
> 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.
> 
>    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.

> 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 integration
>    with the VM, but probably would benefit from it.  The only remotely
>    close example is vbox, but I usually avoid reading that code.
> 
>    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.
> 
>    With the use of the listed facilities, ported driver code 'feels' like
>    FreeBSD code, not Linux code.  Mix of the Linux KPI-compatible wrappers
>    for shallow facilities, and native interfaces for anything more
>    involved, makes the code which is hard to understand for either FreeBSD
>    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.

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

-- 
Jean-Sébastien Pédron

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 949 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20140908/89a3ef6a/attachment.sig>