Re: [PATCH 08/12] linux-user: Add support for setting alsa timer enhanced read using ioctl

[Aleksandar Markovic <aleksandar.m.mail@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4940 bytes --]

On Thursday, January 16, 2020, Aleksandar Markovic <
aleksandar.m.mail@gmail.com> wrote:

>
>
> On Wednesday, January 15, 2020, Laurent Vivier <laurent@vivier.eu> wrote:
>
>> Le 15/01/2020 à 20:17, Filip Bozuta a écrit :
>> >
>> > On 15.1.20. 17:37, Arnd Bergmann wrote:
>> >> On Wed, Jan 15, 2020 at 5:32 PM Laurent Vivier <laurent@vivier.eu>
>> wrote:
>> >>> Le 15/01/2020 à 17:18, Arnd Bergmann a écrit :
>> >>>> On Wed, Jan 15, 2020 at 4:53 PM Filip Bozuta
>> >>>> <Filip.Bozuta@rt-rk.com> wrote:
>> >>>>> This patch implements functionality of following ioctl:
>> >>>>>
>> >>>>> SNDRV_TIMER_IOCTL_TREAD - Setting enhanced time read
>> >>>>>
>> >>>>>      Sets enhanced time read which is used for reading time with
>> >>>>> timestamps
>> >>>>>      and events. The third ioctl's argument is a pointer to an
>> >>>>> 'int'. Enhanced
>> >>>>>      reading is set if the third argument is different than 0,
>> >>>>> otherwise normal
>> >>>>>      time reading is set.
>> >>>>>
>> >>>>> Implementation notes:
>> >>>>>
>> >>>>>      Because the implemented ioctl has 'int' as its third argument,
>> >>>>> the
>> >>>>>      implementation was straightforward.
>> >>>>>
>> >>>>> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
>> >>>> I think this one is wrong when you go between 32-bit and 64-bit
>> >>> kernel uses an "int" and "int" is always 32bit.
>> >>> The problem is most likely with timespec I think.
>> >>>
>> >>>> targets, and it gets worse with the kernel patches that just got
>> >>>> merged for linux-5.5, which extends the behavior to deal with
>> >>>> 64-bit time_t on 32-bit architectures.
>> >>>>
>> >>>> Please have a look at
>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-n
>> ext.git/log/?h=80fe7430c70859
>> >>>>
>> >>> Yes, we already had the same kind of problem with SIOCGSTAMP and
>> >>> SIOCGSTAMPNS.
>> >>>
>> >>> Do the kernel patches add new ioctl numbers to differentiate 32bit and
>> >>> 64bit time_t?
>> >> Yes, though SNDRV_TIMER_IOCTL_TREAD is worse: the ioctl argument
>> >> is a pure 'int' that decides what format you get when you 'read' from
>> the
>> >> same file descriptor.
>> >>
>> >> For emulating 64-bit on 32-bit kernels, you have to use the new
>> >> SNDRV_TIMER_IOCTL_TREAD64, and for emulating 32-bit on
>> >> 64-bit kernels, you probably have to return -ENOTTY to
>> >> SNDRV_TIMER_IOCTL_TREAD_OLD unless you also want to
>> >> emulate the read() behavior.
>> >> When a 32-bit process calls SNDRV_TIMER_IOCTL_TREAD64,
>> >> you can translate that into the 64-bit
>> >> SNDRV_TIMER_IOCTL_TREAD_OLD.
>> >>
>> >>       Arnd
>> >
>> >
>> > Thank you for bringing this up to my attention. Unfortunately i have
>> > some duties of academic nature in next month so i won't have much time
>> > fix this bug. I will try to fix this as soon as possible.
>>
>> Could you at least to try to have a mergeable series before you have to
>> stop to work on this?
>>
>> You can only manage the case before the change reported by Arnd (I will
>> manage the new case myself later).
>>
>>
> Hi, all.
>
> Sorry for interjecting myself into this discussion, but I just want to let
> you know about some related practicalities.
>
> Filip is a student that is from time to time (usually between two exam
> seasons) an intern in our company, working 4 hours each work day. He spent
> his internship in different teams in prevous years, and, from around
> mid-October 2019, was appointed to QEMU team. After some introductory
> tasks, he was assigned his main task: linux-user support for RTCs and ALSA
> timers. This series is the result of his work, and, to my great pleasure,
> is virtually entirely his independant work. I am positive he can complete
> the series by himself, even in the light of additional complexities
> mentioned in this thread.
>
> However, his exam season just started (Jan. 15th), and lasts till Feb.
> 15th. Our policy, in general, is not to burden the students during exam
> seasons, and that is why we can't expect prompt updates from him for the
> time being.
>
> In view of this, Laurent, please take Filip's status into consideration.
> As far as mergeability is concerned, my impression is that patches 1-6 and
> 13 are ready for merging, while patches 7-12 would require some additional
> (netlink-support-like) work, that would unfortunately be possible only
> after Feb. 15th.
>
> Best wishes,
> Aleksandar
>
>
>
Laurent, hi again.

I am not completely familiar with all details of Filip's work, since, as I
said, he had large degree of independance (which was intentional, and is a
desired and good thing IMHO), but taking a closer look, a question starting
lingering: Do we need special handling od read() even for RTC devices - not
only ALSA timer devices?

Best regards,
Aleksandar


>
>
> Thanks,
>> Laurent
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 6810 bytes --]