[PATCH libinput 0/1] Initialization race against udev

Thu Apr 5 13:44:29 UTC 2018

On Thu, 5 Apr 2018 13:20:51 +0000
"Friedrich, Eugen (ADITG/ESB)" <efriedrich at de.adit-jv.com> wrote:

> Hi Peter
> 
> Best regards
> 
> Eugen Friedrich
> Engineering Software Base (ADITG/ESB)
> 
> Tel. +49 5121 49 6921
> 
> > -----Original Message-----
> > From: wayland-devel [mailto:wayland-devel-
> > bounces at lists.freedesktop.org] On Behalf Of Peter Hutterer
> > Sent: Donnerstag, 5. April 2018 06:27
> > To: Matt Hoosier
> > Cc: Pekka Paalanen; wayland mailing list
> > Subject: Re: [PATCH libinput 0/1] Initialization race against udev
> > 
> > On Wed, Apr 04, 2018 at 09:04:32AM -0500, Matt Hoosier wrote:  
> > > I encounter another "bad" behavior related to multiple firings of the UDev
> > > 'add' event for a given input node.
> > >
> > > Typically on modest embedded systems, you do not want to run the  
> > exhaustive  
> > > 'udevadm trigger' during the main booting sequence. That causes hundreds  
> > or  
> > > thousands of UDev nodes to be crawled and processed by udevd all during  
> > the  
> > > first moments of userspace. These overall bulk of these nodes are pretty
> > > much irrelevant to the use-case of the device. The time spent processing
> > > these non-essential /sys devices can easily slow down the device's  
> > progress  
> > > toward starting a UI with basic touch support and loading drivers for the
> > > handful of essential peripherals by 10 or 15 seconds.
> > >
> > > Instead, it works better to manually command the same UDev coldplugging
> > > that would have been done by 'udevadm trigger', but for a very small
> > > hand-picked set of devices rather than everything in /sys/devices. The time
> > > savings are large. Of course, for completeness you do eventually have to
> > > run 'udevadm trigger' so that the full set of hardware and kernel software
> > > features are activated, but that can wait until after the main KPIs are
> > > achieved.
> > >
> > > This scheme generally works just fine. Manually stimulating udevd to
> > > coldplug the specific devices you need keeps everything general:
> > > applications that find their hardware with UDev (such as with libinput or
> > > Weston's DRM backend) all get to rely on their usual well-tested  
> > codepaths.  
> > >
> > > But there is a snag: if a device like /dev/input/event0 has been
> > > coldplugged once with the hands-on technique, then all the daemons that
> > > care about it have already seen one UDev ACTION=add event for it. When  
> > the  
> > > late-running 'udevadm trigger' does its exhaustive sweep across
> > > /sys/devices, this will cause a second ACTION=add event to be triggered  
> > for  
> > > /dev/input/event0. Currently (well, with libinput 1.1.1) this causes
> > > libinput -- and consequently Weston -- to open a second filedescriptor
> > > against /dev/input/event0, so that all input events are received in
> > > duplicate. That confuses the compositor's and applications' input event
> > > handling.
> > >
> > > Would it be tolerable to put a patch into either libinput or Weston to
> > > guard against double-opening the same input device? I realize that the
> > > scheme outlined above is probably technically in violation of the expected
> > > UDev initialization scheme, but I'm not aware of any way to suppress  
> > udevd  
> > > from broadcasting the 'add' action to all udev clients even though the
> > > device has already been coldplugged once. It seems to me at least plausible
> > > that the Weston stack would benefit from guarding against getting into this
> > > bad state from receiving unexpected UDev events.  
> > 
> > 
> > I don't think a weston patch is likely to help you here since libinput's
> > udev handling is independent of weston. but tbh, I'd rather not do this in
> > libinput either. as you write, you're not really behaving like udev is
> > supposed to and once we start having to deal with that the floodgates are
> > open for weird patches. The code for this should be relatively
> > self-contained and easy-enough to maintain, please maintain that as a
> > distribution-specific patch.  
> 
> [EF] but as I can see there is a race condition which can always
> occur in any system if device will be added to the system after
> installation of the  udev monitor and before initializing already
> coldplugged devices. Can we really just accept this and hope it will
> never happen? Or I'm missing something?

Hi,

yup, these are two different problems. The problem my patch solves is
exactly what you describe. It has been merged already.

The problem Matt is talking about is manually triggered duplicate udev
device "add" events, because he wants to first add some devices and
only later do the full cold-plug. Since the cold-plug (presumably, or
at least currently) cannot know which devices were already "add"
advertized, it sends "add" for them all, resulting in duplicates. This
happens only because udevadm is ran manually to do cold-plug
essentially twice. It's not a race even.

Peter's second suggestion, as I understood, was to install temporary
udev rules after the partial cold-plug trigger to prevent libinput from
handling duplicated adds during the full cold-plug. You'd also need to
remove those rules after the full cold-plug finished, so that if the
partial-cold-plugged devices are physically removed and re-plugged,
they would not get ignored again.

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180405/b71c1eb1/attachment.sig>