| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |
| Who | What | When |
|---|---|---|
| *** | ChanServ sets mode: +v mchehab | [03:23] |
| ....................................................................................................... (idle for 8h34mn) | ||
| mchehab | hi all | [11:57] |
| hverkuil | hi | [11:58] |
| syoung | hi | [12:00] |
| mchehab | anything for today's meeting? | [12:03] |
| hverkuil | I'm very close to adding a regression test to the daily build.
Hope to finish it today. | [12:04] |
| mchehab | nice | [12:05] |
| hverkuil | Still waiting for reviews of "[RFCv2 PATCH] videodev2.h: introduce VIDIOC_DQ_EXT_EVENT" | [12:06] |
| mchehab | sorry... have been busy those days with some other stuff | [12:06] |
| sailus | Ahoy! | [12:07] |
| hverkuil | pinchartl: ping | [12:07] |
| mchehab | one thing I didn't like was the addition of reserved stuff there... IMHO, we should not add reserved fields anymore, and instead use the ioctl size when new fields get added
(but didn't really review the patch... just did a real quick look on it) | [12:08] |
| sailus | hverkuil: I looked at it but forgot to reply. It seems good to me, but please wait for my reply. | [12:09] |
| hverkuil | I'm not keen on that at all. I find it very painful to support that, both in the kernel and in userspace. An application will have to try all the variants of the ioctl, see which is supported by the kernel. | [12:09] |
| pinchartl | bonjour
hverkuil: pong | [12:09] |
| sailus | pinchartl: Huomenta! | [12:10] |
| hverkuil | pinchartl: are you planning to make a pull request today/tomorrow? There are a few patches I really would like to get in for 5.1. | [12:10] |
| pinchartl | hverkuil: was that for Mauro ? | [12:11] |
| hverkuil | pinchartl: "uvc: fix smatch warning", "uvc: use usb_make_path to fill in usb_info" and "vsp1: fix smatch warning" | [12:11] |
| sailus | hverkuil: One way would be to opportunistically ignore the part of the IOCTL argument that isn't known to the kernel to mirror the current use of the reserved fields.
I vaguely remember we decided against that a few years back though. | [12:12] |
| hverkuil | pinchartl: there are patches delegated to you, but I would like to get them merged for 5.1: clean up of smatch warnings and fixing the bus_info for uvc (makes regression testing easier). | [12:12] |
| pinchartl | I was planning to send a pull request for v5.2 as -rc6 is out already | [12:13] |
| mchehab | at Kernel side, supporting multiple versions of an ioctl seem easy | [12:13] |
| hverkuil | pinchartl: Mauro closes tomorrow or Monday.
mchehab: kernel side is easy, it's the impact on userspace that's the problem. | [12:13] |
| pinchartl | I have no objection against them going in v5.1
hverkuil: if you have another pull request planned it would be easier if you included them there, as I have nothing else on my side | [12:14] |
| hverkuil | pinchartl: OK, I'll delegate these three patches to me and make a PR today or tomorrow. | [12:15] |
| mchehab | hverkuil: the only impact it may have is if someone compiles an app against a newer kernel to use on an older kernel.... only test apps do that | [12:15] |
| pinchartl | well, I guess there are a couple of other non-urgent patches I could include in there
hverkuil: I'll do it then | [12:15] |
| hverkuil | pinchartl: OK :-) | [12:15] |
| mchehab | those need to have fallback code anyway, as they should check if the new fields were used or not by the Kernel | [12:15] |
| hverkuil | mchehab: not true: I can switch to older kernels in grub if something is unstable. I have done so in the past, and will do so in the future. | [12:16] |
| sailus | mchehab: I mostly agree, albeit you could theoretically run into that with containers. But typically V4L2 applications aren't run on containers. | [12:16] |
| hverkuil | Also, most applications make a copy of videodev2.h and compile against that.
You really cannot make any assumptions about this! | [12:17] |
| sailus | hverkuil: How about just ignoring the part of the argument not known to the kernel?
That's more or less equivalent to what we do with reserved fields now. | [12:17] |
| pinchartl | hverkuil: if an application is compiled against newer headers and run on an older kernel, the kernel will ignore the extra fields, so that's not a problem | [12:19] |
| hverkuil | and zero the extra data when returning from the ioctl? | [12:19] |
| pinchartl | (we may want to zero them instead though) | [12:19] |
| hverkuil | If we do this, then this is a massive change from the current way the API works. I am not at all certain this is a good idea. | [12:21] |
| pinchartl | haven't we discussed this in the past ?
it's commonly used in other subsystems | [12:21] |
| hverkuil | if you can point me to an implementation in another subsystem, then I can take a look what they do there. | [12:21] |
| mchehab | hverkuil: basically, at read, it would copy just up to _IOC_SIZE(cmd) and zero the remaining space; at return, it will just copy up to _IOC_SIZE(cmd)
seems easy to implement and support and will have the same effect as if we had reserved fields | [12:22] |
| pinchartl | hverkuil: drm_ioctl() | [12:25] |
| hverkuil | It would have to be restricted to ioctls where this will work (basically ioctls that don't need compat32 code or have y2038 layout differences) | [12:25] |
| pinchartl | that will be the case for all our new ioctls, right ? | [12:26] |
| mchehab | I'm not proposing changing the existing ioctls... just for new ones
and of course they should use 64 bits pointers, in order to avoid compat32 code | [12:26] |
| hverkuil | Hmm, that will work.
OK, just assume I'll make the necessary changes for this in v4l2-ioctl.c and ignore the reserved fields. when reviewing that patch. | [12:27] |
| mchehab | OK
I'll probably add a comment there at review just as a reminder | [12:29] |
| hverkuil | What I would really like to do long-term is to have replacements for all the ioctls that need special 32 bit handling today that are compat32 clean.
Those conversions are very painful and inefficient. Combined with this proposal it would make sense. | [12:29] |
| mchehab | I don't see any gain on replacing them just due to compat32...
I mean: we'll need to keep the code there forever and having 32-bits userspace, 64-bits kernelspace is something that will disappear over time | [12:31] |
| hverkuil | BTW, for 5.2 I would like to move all the drivers/media/media-* sources into a new subdirectory drivers/media/media or perhaps media/mc. These media sources are starting to clutter this directory. | [12:32] |
| mchehab | I remember I saw some discussions recently about removing 32-bits userspace on x86 - this didn't happen yet, but will probably happen some day
(the same will likely happen on arm - long term) | [12:33] |
| hverkuil | very long term :-) | [12:33] |
| sailus | mchehab: That was probably x32, not i386. | [12:33] |
| mchehab | sailus: I guess both... distro people just don't want to support i386 anymore... too much effort for too little people using it | [12:34] |
| sailus | Oh really?
I have a virtual machine using i386. | [12:36] |
| mchehab | anyway, to be frank, I don't think it is worth the time to redesign ioctls just due to compat32 | [12:36] |
| sailus | And a real one in the cellar. X-) | [12:36] |
| mchehab | sailus: I don't remember where I heard, but I'm pretty sure I read something about that recently | [12:37] |
| sailus | Some recent x86 cores are 32-bit but are not really intended to be run Linux. | [12:37] |
| mkrufky | eek! no more i386?!? | [12:37] |
| hverkuil | mchehab: I think it was about x32, I read that as well | [12:37] |
| mchehab | it should take some time to happen (and will likely happen at distros first) | [12:37] |
| sailus | x32 is pretty neat IMHO. I would like to use it if it was viable. | [12:38] |
| mchehab | x32 arch removal was an upstream discussion | [12:38] |
| hverkuil | I don't see i686 going away for a very long time.
Perhaps for distros, but not kernel support. | [12:38] |
| mchehab | but I think I saw some discussions from a distro forum too
for the i386/... packages https://itsfoss.com/ubuntu-drops-32-bit-desktop/ (this is one of such discussions - it was not the one I read on that time though) | [12:38] |
| sailus | mchehab: Ubuntu folks did that decision quite a while ago while e.g. Debian still ships the i386 port. | [12:41] |
| mchehab | sailus: Debian is LTS
things are slower on LTS distros | [12:41] |
| sailus | So does Slackware! | [12:41] |
| mkrufky | iirc i686 kernels run fine on i386 hardware, no? it's been a while | [12:42] |
| sailus | mchehab: I'm not sure what you're referring to. Recent Debian releases have had extended support though, as well as by third parties once the Debian project itself has dropped the support, but this is not related to the ports for the current stable release.
Um, Gentoo supports 32-bit x86, too. | [12:43] |
| *** | ChanServ sets mode: +v mchehab` | [12:45] |
| sailus | And Debian probably doesn't support i386 anymore; I remember there were issues on C++ programs, and the choice was between supporting i386 (vs. i486) or making C++ programs slower for everyone. | [12:46] |
| mchehab` | sailus: what I am saying is that having a long term plan to get rid of compat32 and rewrite all ioctls doesn't make much sense to me | [12:46] |
| sailus | mchehab`: I agree with that, but it's a different matter. :-) | [12:47] |
| mchehab` | as:
1) the current code will still need to be there; 2) distros are (slowing) removing support for 32 bits on 64-bits archs; | [12:47] |
| sailus | I'd just rework the IOCTLs when there's a need to do that.
That's my current thinking at least. | [12:47] |
| mchehab` | 3) almost all usecases either use 32 or 64 bits - arm64 being perhaps the only exception - and probably only for a while | [12:48] |
| mchehab | sailus: agreed. when we need a new ioctl redesign, let's do it using Y2038-safe code, 64bits-aligned and not requiring 32-bits compat stuff | [12:51] |
| hverkuil | mchehab: ack
For me DQEVENT is a first step, one that isn't too complex. Next up will be the streaming I/O ioctls (struct v4l2_buffer), but it is easier to work on those if DQEVENT is done first (both creating a 64 bit aligned, y2038 safe version, and creating the y2038 compatibility code for the old DQEVENT ioctl). | [12:51] |
| mchehab | ok, but let's do it for the right reasons: need to fix Y2038 or support new features (and not just due to compat32) | [12:55] |
| hverkuil | virtme is fantastic to run tests for a new kernel! Really neat and easy. | [12:55] |
| mchehab | anything else for today's discussions? | [12:58] |
| pinchartl | hverkuil: I use qemu + a buildroot fs that I mount through nfsroot | [12:58] |
| hverkuil | not from me
pinchartl: thank you for the PR! | [13:00] |
| sailus | That's all from me. | [13:05] |
| ..... (idle for 23mn) | ||
| pinchartl | hverkuil: you're welcome | [13:28] |
| .......... (idle for 46mn) | ||
| *** | ChanServ sets mode: +v mchehab | [14:14] |
| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |