linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).