linux-kernel - Re: [Hdaps-devel] [PATCH] hdaps - switch to using input-polldev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200707082324.11315.dtor@insightbb.com>
Date:	Sun, 8 Jul 2007 23:24:08 -0400
From:	Dmitry Torokhov <dtor@...ightbb.com>
To:	Shem Multinymous <multinymous@...il.com>
Cc:	hdaps-devel@...ts.sourceforge.net, rlove@...ve.org,
	Linux Kernel ML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...l.org>,
	Michael Riepe <michael@...11.de>,
	Henrique de Moraes Holschuh <hmh@...ian.org>
Subject: Re: [Hdaps-devel] [PATCH] hdaps - switch to using input-polldev

On Sunday 08 July 2007 21:00, Shem Multinymous wrote:
> On 5/25/07, Dmitry Torokhov <dtor@...ightbb.com> wrote:
> > HWMON: hdaps - convert to use input-polldev.
> >
> > Switch to using input-polldev skeleton instead of implementing
> > polling loop by itself. This also fixes problem with trylock
> > on a mutex in atomic context.
> >
> > Signed-off-by: Dmitry Torokhov <dtor@...l.ru>
> 
> There's a couple of inherent problems with this patch (now in -mm).
> 
> First, the hdaps driver regularly polls the embedded controller, which
> in turns regularly polls the hardware. If the two polling rates differ
> or fluctuate, we lose events.

That was the case with the original driver as well bit instead of
rearming workqueue it was using rearming timer.

> AFAICT, the delayed workqueues used by 
> input-polldev can get very laggy under load. That's very bad for
> sensitive clients like hdapsd (the hard disk shock protection daemon).
>

input-polldev uses a separate workqueue, not keventd, and so should not
suffer from other workqueue users loading keventd. But if entire box
is under stress then workqueue vs timer context does not matter much -
your daemon which is in userspace may not get to run in a timely manner
anyway.

However I am open to bumping up priority of ipolldevd a little.
 
> Second, this is incompatible with the much-needed addition of a 2nd
> input device relying on the same data. The existing hdaps input device
> does "joystick emulation", i.e., reports values after calibration and
> fuzzing. Userspace programs that need the raw data, like hdapsd,
> currently have to poll the sysfs attribute, which is inefficient,
> lag-prone and induces unnecessary interrupts on tickless sytems. To
> solve this we'll have to add a 2nd input device to hdaps, for
> reporting the raw accelerometer data. (Michael Riepe and me are now
> working on such a patch.) But these two input devices need to share
> their polling of the underlying EC hardware, and this is impossible
> using input-polldev.

I am curious why you can't use the current device, since the calibration
done in hdaps does not alter the scale but merely moves '0' point around.
And fuzz should only remove small jitters, not rapidly changing data
that you shoudl get when your box is falling.
 
However nothing stops you from generating events for the 2nd input
device from the same polling function that generates events for the
first device.

> 
> Since this patch will  degrade accuracy and will eventually be
> reverted anyway, I suggest retracting it.

I have not seen anything in your mail that would warrant reverting
the patch.

> 
> As for the mutex in atomic context issue, isn't it best addressed by
> making mutex_trylock() do the sensible thing in softirqt?
> 
>   Shem
> 

-- 
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/