From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Chase Douglas <chase.douglas@canonical.com>
Cc: Benjamin Tissoires <benjamin.tissoires@gmail.com>,
Henrik Rydberg <rydberg@euromail.se>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] Input: evdev - Add EVIOC mechanism to extract the MT slot state
Date: Fri, 6 Jan 2012 12:17:36 -0800 [thread overview]
Message-ID: <20120106201735.GA23818@core.coreip.homeip.net> (raw)
In-Reply-To: <4F075500.3020601@canonical.com>
On Fri, Jan 06, 2012 at 12:09:36PM -0800, Chase Douglas wrote:
> On 01/06/2012 11:58 AM, Dmitry Torokhov wrote:
> > On Fri, Jan 06, 2012 at 10:56:46AM -0800, Chase Douglas wrote:
> >> On 01/06/2012 10:18 AM, Dmitry Torokhov wrote:
> >>> Hi Benjamin,
> >>>
> >>> On Fri, Jan 06, 2012 at 07:00:22PM +0100, Benjamin Tissoires wrote:
> >>>> Hi guys,
> >>>> I read somewhere in the code of Android a comment in which they
> >>>> complain about not being able to retrieve the slots states. So they
> >>>> assume they are all at 0.
> >>>> So this mechanism is good to have.
> >>>> However, back in January 2011, Dmitry raised the problem that this
> >>>> code was not thread safe.What happens if 2 applications ask for
> >>>> different slots values (let say X.org and utouch-frame)?
> >>>
> >>> 2 different processes should be fine; the problem would be if 2 threads
> >>> of the same process share the same file descriptor. So far the rest of
> >>> evdev copes just fine with multiple threads using the same fd (all
> >>> operations are atomic in this regard), setting ABS_MT_SLOT before
> >>> fetching the state break this property.
> >>
> >> How is this any different than two threads trying to set a different
> >> property, like the fuzz factor of an axis? This seems like something
> >> that should be guarded by a lock in userspace, essentially.
> >
> > From kernel POV both operations succeed and produce consistent reults.
> > Consider EVIOCSABS when one thread using the same FD sets range 0-100
> > and another 200-1000. At no time in the kernel we get to state of
> > min = 200 and max = 1000. In the end we'll end up with either 0-100 or
> > 200-1000 but not mix of both. So the kernle state is internally
> > consistent.
>
> I don't see how modifying the slot requested could ever get the kernel
> into an inconsistent state.
It may cause client get data that it did not request. In other worse it
kernel may supply wrong data to the caller.
>
> > With proposed solution one client may request data for slot 2 but
> > instead get info for slot 5 if another client manages to slide in.
>
> You can do the same thing with EVIOCSABS. If you don't do proper locking
> and handling, two threads can assume they wrote a value to evdev and it
> was successful, when in reality only the second thread to make the call
> has any effect.
As with pretty much any other resource; but there is a reason we have
atomic variables and operations. The distinction is that both operations
carried out completely and consistently.
>
> I know there's a slight distinction between these two scenarios, but my
> point is that if you are doing multithreaded evdev reading from the same
> evdev fd, you are asking for trouble and you need to be careful. That
> even goes for modifying any of the other state through EVIOCSABS from
> multiple processes. And really, how many programs are out there reading
> from the same evdev fd in multiple threads. I'd wager a fair amount of
> money the answer is 0.
I am really not concerned about what userspace might do - I've looked at
enough code to see all kinds of weird stuff. My task is to make sure
that kernel interface is sane and since it is userspace ABI matter I
want to be extra careful.
Thanks.
--
Dmitry
next prev parent reply other threads:[~2012-01-06 20:17 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-06 15:16 [PATCH v2] Input: evdev - Add EVIOC mechanism to extract the MT slot state Henrik Rydberg
2012-01-06 16:25 ` Chase Douglas
2012-01-06 18:00 ` Benjamin Tissoires
2012-01-06 18:18 ` Dmitry Torokhov
2012-01-06 18:55 ` Henrik Rydberg
2012-01-06 19:18 ` Henrik Rydberg
2012-01-06 19:34 ` Chase Douglas
2012-01-06 19:48 ` Henrik Rydberg
2012-01-06 20:02 ` Dmitry Torokhov
2012-01-06 20:14 ` Henrik Rydberg
2012-01-06 20:23 ` Dmitry Torokhov
2012-01-06 20:30 ` Henrik Rydberg
2012-01-06 20:03 ` Dmitry Torokhov
2012-01-06 18:56 ` Chase Douglas
2012-01-06 19:58 ` Dmitry Torokhov
2012-01-06 20:09 ` Chase Douglas
2012-01-06 20:17 ` Dmitry Torokhov [this message]
2012-01-06 20:44 ` Chase Douglas
2012-01-06 18:45 ` Henrik Rydberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120106201735.GA23818@core.coreip.homeip.net \
--to=dmitry.torokhov@gmail.com \
--cc=benjamin.tissoires@gmail.com \
--cc=chase.douglas@canonical.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rydberg@euromail.se \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).