[PATCH weston v2 3/4] libweston: Add check and fallback for linux/sync_file.h

[Christian Stroetmann]

Hello everybody

On 27.09.2017 08:17, Alexandros Frantzis wrote:
> On Tue, Sep 26, 2017 at 02:07:45PM +0300, Pekka Paalanen wrote:
>> On Tue, 19 Sep 2017 14:59:10 +0300
>> Alexandros Frantzis<alexandros.frantzis at collabora.com>  wrote:
>>
>>> The sync file functionality is required by the upcoming GPU render
>>> timeline work, but it's only available in relatively new linux kernel
>>> versions (4.7 and above).
>>>
>>> This commit provides an in-tree copy of the required sync file
>>> definitions.  On systems that don't have the sync file header (due to
>>> having an older kernel), we will be able to fall back to our own copy
>>> when building.
>>>
>>> Signed-off-by: Alexandros Frantzis<alexandros.frantzis at collabora.com>
>>> ---
>>>
>>> Changes in v2:
>>>    - New patch.
>>>
>>>   Makefile.am                  |  1 +
>>>   configure.ac                 |  1 +
>>>   libweston/weston-sync-file.h | 73 ++++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 75 insertions(+)
>>>   create mode 100644 libweston/weston-sync-file.h
>>> diff --git a/libweston/weston-sync-file.h b/libweston/weston-sync-file.h
>>> new file mode 100644
>>> index 00000000..4d179df0
>>> --- /dev/null
>>> +++ b/libweston/weston-sync-file.h
>>> @@ -0,0 +1,73 @@
>>> +/*
>>> + * Copyright (C) 2012 Google, Inc.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>> Hi,
>>
>> I'm slightly nervous about this note about GPL. I assume we should be
>> just fine with it, just have the comment below explain how this doesn't
>> make Weston GPL, but on the other hand it actually looks quite easy to
>> avoid this completely.
>>
>> Looking at the code in patch 4, we should be fine with just #ifdeffing
>> the contents of linux_sync_file_read_timestamp() and return -1 when the
>> header was not present.
>>
>> I don't think we would necessarily need to bother with disabling the
>> GPU timestamping on start-up based on header availability. It will just
>> always fail on reading the timestamp which it would also do if the
>> kernel was too old. And maybe EGL already does the kernel runtime
>> checks for us?
>>
>>> + *
>>> + */
>>> +/* Sync file interface used by Weston, copied from kernel's sync_file.h */
>> If we really want to keep the fallback definition instead, I would
>> expand on the comment you added:
>>
>> /* Sync file kernel UABI definitions used by Weston, copied from
>>   * the Linux kernel version x.xx include/uapi/linux/sync_file.h
>>   * This is needed for interfacing with the kernel and does not
>>   * imply the GNU GPL licence on Weston.
>>   */
>>
>>
>> But, do people expect to be able to use kernel features that are not
>> present in the kernel headers used for building the userspace?
> The same question has been implicitly asked for EGL, too,
> and the answer there seems to be that we care about building on systems
> that don't have all the EGL functionality Weston may use at runtime. I
> would expect the same answer to apply to the kernel too, when possible.
>
> All that being said, I understand the reluctance to include a copy of
> the kernel userspace API header, since it's a grey area in terms of GPL.
> Google did a similar thing for bionic a few years ago, and although this
> move caused some concern, no definitive legal answer was given (bionic
> still uses a striped down version of the headers without issue today).
> It's unfortunate that the situation is not clearer.

A definitive legal answer was given. The stripped down version of the 
headers is covered by the fair use clause of the copyright because it is 
arbitrary code respectively it is not sufficiently original and unique, 
and not directly connected with the individual creative expression of 
the authors of Linux.
Or said in other words, the stripped down version of the headers include 
common code of operating systems and when you read the stripped down 
version of the headers, then you are unable to confuse the origin of the 
code and to decide if it was written by the Linux authors, Google, or 
another entity.
Exactly for creating this legal situation the headers were stripped down 
from such lines of code that would have revealed its origin respectively 
show the connection with the authors of Linux.

This trick does not work for every header or other parts of software. If 
a header defines a very specific item, for example an array of 7 very 
special items that is directly connected with the rest of an author's 
code, work respectively individual expression, then even a header 
consisting of some few lines is protected by the copyright.

>> Seeing how simple the #ifdef solution is, personally I would prefer it
>> over the fallback copy of sync_file.h.
> As mentioned above, I don't think it's ideal from a practical
> perspective, but I agree it's probably the simplest way to move forward
> (without involving lawyers :)). I will respin the patchset to include
> this change (and also some changes discussed for patch 4).
>
>>> +
>>> +#ifndef WESTON_SYNC_FILE_H
>>> +#define WESTON_SYNC_FILE_H
>>
>> Thanks,
>> pq
> Thanks,
> Alexandros
>

Best regards
Christian Stroetmann