Re: [RFC] libgpiod public API reviews needed

[Bartosz Golaszewski <brgl@xxxxxxxx>]

2018-01-22 10:25 GMT+01:00 Arnd Bergmann <arnd@xxxxxxxx>:
> On Mon, Jan 22, 2018 at 9:21 AM, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>> On Sun, Jan 21, 2018 at 11:18 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>>> On Sun, Jan 21, 2018 at 10:30 PM, Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>>>> 2018-01-21 16:49 GMT+01:00 Linus Walleij <linus.walleij@xxxxxxxxxx>:
>>>>> On Fri, Jan 19, 2018 at 2:28 PM, Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>>>
>>>> I was not aware of this, but it seems you're right! Nice catch, thanks.
>>>>
>>>> How about defining a local struct gpiod_timespec with both seconds and
>>>> nanoseconds explicitly defined to uint64_t?
>>>
>>> Where is that timestamp generated? Is this purely a user space interface
>>> with the time read from gettimeofday(), or are we talking about  a new
>>> kernel-to-user interface?
>>
>> This is in include/uapi/linux/gpio.h:
>>
>> /**
>>  * struct gpioevent_data - The actual event being pushed to userspace
>>  * @timestamp: best estimate of time of event occurrence, in nanoseconds
>>  * @id: event identifier
>>  */
>> struct gpioevent_data {
>>         __u64 timestamp;
>>         __u32 id;
>> };
>>
>> It is the same as is used for IIO. Inside the kernel this ultimately
>> comes from ktime_get_real_ns();
>
> Ah, too bad, that already contains two mistakes:
>
> - on x86, the structures are incompatible between 32-bit and 64-bit
>   user space, as the former has no padding.

Is this really an issue? Do distros really ship the same bytecode for
32 and 64 bit architecures? I have never run into such problems
despite having used different python bindings for C libraries (I'm not
sure however how many of them dealt with any visible C structs).

> - 'real' timestamps are inconvenient because time may jump in
>   either direction. Time stamps should use 'monotonic' time, i.e.
>   ktime_get_ns().
>

@Linus: this doesn't really break the ABI - how do you feel about
switching to using it in gpiolib.c?

>>> In a lot of cases, a simple 64-bit nanosecond counter using CLOCK_MONOTONIC
>>> timestamps is the most robust and simple solution.
>>
>> Bartosz also seems to think it is the best so would vote to go
>> for that and we have one problem less.
>
> Could we introduce a new ioctl to replace the gpioevent_data() and
> use a better interface then?
>

For the security concern - I guess it would be enough to just zero
gpioevent_data in lineevent_irq_thread() before putting it into the
FIFO?

Best regards,
Bartosz Golaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html