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

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

[Shem Multinymous]

On 5/25/07, Dmitry Torokhov <dtor@insightbb.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@mail.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. 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).

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.

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

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

  Shem

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

[Dmitry Torokhov]

On Sunday 08 July 2007 21:00, Shem Multinymous wrote:
> On 5/25/07, Dmitry Torokhov <dtor@insightbb.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@mail.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

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

[Shem Multinymous]

Hi Dmitry,

On 7/8/07, Dmitry Torokhov <dtor@insightbb.com> wrote:
> > 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.

Right. Doesn't the latter result in more regular scheduling?


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

The daemon itself typically runs with a higher priority (and sleeps a
lot so it gets further dumped). More importantly, the daemon depends
not only on the latest measurement, but also on recent measurements
have been obtained from the hardware in a regular fashion and with
reasonably accurate timestamps. And *this* depends solely on the hdaps
driver.


> However I am open to bumping up priority of ipolldevd a little.

Will this result in scheduling tha'ts as reliable as rearming timers
from softirq? I saw claims to the contrary, but it it's true then I
withdraw the first objection.


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

Recent versions of the hdapsd daemons do much more than a simple
threshold check: they gather some 2nd-order and decaying averages
statistics to catch subtle abnormal movement (e.g., sliding off a
surface) that's indicative of potential shock. As pointed out in IBM's
HDAPS whitepaper, by the time the box is actually in free fall, it's
too late to start parking the heads. Now, that kind of movement is not
very far from the noise floor, so hdapsd needs all the accuracy it can
get -- hence fuzzing is very disruptive. Calibration is currently
harmless, but I can certainly imagine more advanced hdapsd that uses
heuristics based, e.g., on the absolute orientation of the laptop, so
let's not ruin this data.


> However nothing stops you from generating events for the 2nd input
> device from the same polling function that generates events for the
> first device.

You could one input device open, or the other, or both. How would you
set up input-polldev to handle this?


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

BTW, I think that's worth fixing in any case.

  Shem

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

[Dmitry Torokhov]

On Monday 09 July 2007 00:31, Shem Multinymous wrote:
> Hi Dmitry,
> 
> On 7/8/07, Dmitry Torokhov <dtor@insightbb.com> wrote:
> > > 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.
> 
> Right. Doesn't the latter result in more regular scheduling?
>

Probably.
 
> 
> > > 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.
> 
> The daemon itself typically runs with a higher priority (and sleeps a
> lot so it gets further dumped). More importantly, the daemon depends
> not only on the latest measurement, but also on recent measurements
> have been obtained from the hardware in a regular fashion and with
> reasonably accurate timestamps. And *this* depends solely on the hdaps
> driver.
>

Every input event carries a timestamp so even if there are irregularities
in taking the samples you should be able to account for it. 

> 
> > However I am open to bumping up priority of ipolldevd a little.
> 
> Will this result in scheduling tha'ts as reliable as rearming timers
> from softirq? I saw claims to the contrary, but it it's true then I
> withdraw the first objection.

Probably not. But I still think that if system is so busy that it can't
get aroung to schedule one of workqueues it will not be able to part
the driver fast enough anyway.

> 
> > > 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.
> 
> Recent versions of the hdapsd daemons do much more than a simple
> threshold check: they gather some 2nd-order and decaying averages
> statistics to catch subtle abnormal movement (e.g., sliding off a
> surface) that's indicative of potential shock. As pointed out in IBM's
> HDAPS whitepaper, by the time the box is actually in free fall, it's
> too late to start parking the heads. Now, that kind of movement is not
> very far from the noise floor, so hdapsd needs all the accuracy it can
> get -- hence fuzzing is very disruptive. Calibration is currently
> harmless, but I can certainly imagine more advanced hdapsd that uses
> heuristics based, e.g., on the absolute orientation of the laptop, so
> let's not ruin this data.

If hdaps is the main consumer for the data it may be a good idea to
just remove the fuzz setting from input device. I don't have the hardware,
how bad is it without fuzz?

> 
> 
> > However nothing stops you from generating events for the 2nd input
> > device from the same polling function that generates events for the
> > first device.
> 
> You could one input device open, or the other, or both. How would you
> set up input-polldev to handle this?
> 

Have 2nd input device's ->open() method call input_open_device() for
the first one.

> 
> > > As for the mutex in atomic context issue, isn't it best addressed by
> > > making mutex_trylock() do the sensible thing in softirqt?
> 
> BTW, I think that's worth fixing in any case.
> 
>   Shem
> 

-- 
Dmitry

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

[Shem Multinymous]

On 7/9/07, Dmitry Torokhov <dtor@insightbb.com> wrote:
> > > 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.
> >
> > The daemon itself typically runs with a higher priority (and sleeps a
> > lot so it gets further dumped). More importantly, the daemon depends
> > not only on the latest measurement, but also on recent measurements
> > have been obtained from the hardware in a regular fashion and with
> > reasonably accurate timestamps. And *this* depends solely on the hdaps
> > driver.
> >
>
> Every input event carries a timestamp so even if there are irregularities
> in taking the samples you should be able to account for it.

The issue is how good are the input event timestamps. The way it works
is that the EC samples the analog sensor at some fixed rate and makes
them available over the LPC bus. If the hdaps driver consumes these
samples at the same rate then the timestamps will be accurate up to a
small phase difference, which is mostly inconsequential. But if the
hdaps driver gets scheduled irregularly, its timestamps will be offset
by varying amounts, which will completely throw off computation (e.g.,
think of estimating the angular velocity).


> > > However I am open to bumping up priority of ipolldevd a little.
> >
> > Will this result in scheduling tha'ts as reliable as rearming timers
> > from softirq? I saw claims to the contrary, but it it's true then I
> > withdraw the first objection.
>
> Probably not. But I still think that if system is so busy that it can't
> get aroung to schedule one of workqueues it will not be able to part
> the driver fast enough anyway.

A delay of 20ms in invoking the userspace daemon is negligible, but a
timing variance of 20ms in the driver's measurements is devastating.


> > > 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.
> >
> > Recent versions of the hdapsd daemons do much more than a simple
> > threshold check: they gather some 2nd-order and decaying averages
> > statistics to catch subtle abnormal movement (e.g., sliding off a
> > surface) that's indicative of potential shock. As pointed out in IBM's
> > HDAPS whitepaper, by the time the box is actually in free fall, it's
> > too late to start parking the heads. Now, that kind of movement is not
> > very far from the noise floor, so hdapsd needs all the accuracy it can
> > get -- hence fuzzing is very disruptive. Calibration is currently
> > harmless, but I can certainly imagine more advanced hdapsd that uses
> > heuristics based, e.g., on the absolute orientation of the laptop, so
> > let's not ruin this data.
>
> If hdaps is the main consumer for the data it may be a good idea to
> just remove the fuzz setting from input device. I don't have the hardware,
> how bad is it without fuzz?

People are using the existing input device as a joystick input for
things like Neverball. Current fuzz is 4 and the observed std dev is
roughly 2, so eliminating fuzz will certainly affect the values. The
implications are app-specific, but I guess some apps do care about
such noise, otherwise we wouldn't have fuzz built into the input
infrastructure.


> > You could one input device open, or the other, or both. How would you
> > set up input-polldev to handle this?
>
> Have 2nd input device's ->open() method call input_open_device() for
> the first one.

Won't that create an overhead by the redundant, unused notifications?

  Shem

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

[Dmitry Torokhov]

On Monday 09 July 2007 01:29, Shem Multinymous wrote:
> On 7/9/07, Dmitry Torokhov <dtor@insightbb.com> wrote:
> > > > 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.
> > >
> > > The daemon itself typically runs with a higher priority (and sleeps a
> > > lot so it gets further dumped). More importantly, the daemon depends
> > > not only on the latest measurement, but also on recent measurements
> > > have been obtained from the hardware in a regular fashion and with
> > > reasonably accurate timestamps. And *this* depends solely on the hdaps
> > > driver.
> > >
> >
> > Every input event carries a timestamp so even if there are irregularities
> > in taking the samples you should be able to account for it.
> 
> The issue is how good are the input event timestamps. The way it works
> is that the EC samples the analog sensor at some fixed rate and makes
> them available over the LPC bus. If the hdaps driver consumes these
> samples at the same rate then the timestamps will be accurate up to a
> small phase difference, which is mostly inconsequential. But if the
> hdaps driver gets scheduled irregularly, its timestamps will be offset
> by varying amounts, which will completely throw off computation (e.g.,
> think of estimating the angular velocity).
>

Timers do not guarantee you that they will be fired at the exact time.
If system is under load and there are hard IRQs they will also be
delayed.

> 
> > > > However I am open to bumping up priority of ipolldevd a little.
> > >
> > > Will this result in scheduling tha'ts as reliable as rearming timers
> > > from softirq? I saw claims to the contrary, but it it's true then I
> > > withdraw the first objection.
> >
> > Probably not. But I still think that if system is so busy that it can't
> > get aroung to schedule one of workqueues it will not be able to part
> > the driver fast enough anyway.
> 
> A delay of 20ms in invoking the userspace daemon is negligible, but a
> timing variance of 20ms in the driver's measurements is devastating.
>

Even if you know that there is such variance?
 
> 
> > > > 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.
> > >
> > > Recent versions of the hdapsd daemons do much more than a simple
> > > threshold check: they gather some 2nd-order and decaying averages
> > > statistics to catch subtle abnormal movement (e.g., sliding off a
> > > surface) that's indicative of potential shock. As pointed out in IBM's
> > > HDAPS whitepaper, by the time the box is actually in free fall, it's
> > > too late to start parking the heads. Now, that kind of movement is not
> > > very far from the noise floor, so hdapsd needs all the accuracy it can
> > > get -- hence fuzzing is very disruptive. Calibration is currently
> > > harmless, but I can certainly imagine more advanced hdapsd that uses
> > > heuristics based, e.g., on the absolute orientation of the laptop, so
> > > let's not ruin this data.
> >
> > If hdaps is the main consumer for the data it may be a good idea to
> > just remove the fuzz setting from input device. I don't have the hardware,
> > how bad is it without fuzz?
> 
> People are using the existing input device as a joystick input for
> things like Neverball. Current fuzz is 4 and the observed std dev is
> roughly 2, so eliminating fuzz will certainly affect the values. The
> implications are app-specific, but I guess some apps do care about
> such noise, otherwise we wouldn't have fuzz built into the input
> infrastructure.
>

OK.
 
> 
> > > You could one input device open, or the other, or both. How would you
> > > set up input-polldev to handle this?
> >
> > Have 2nd input device's ->open() method call input_open_device() for
> > the first one.
> 
> Won't that create an overhead by the redundant, unused notifications?
> 

They won't leave input core so nothing really noticeable.


>   Shem
> 

-- 
Dmitry

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

[Shem Multinymous]

Hi,

On 7/9/07, Dmitry Torokhov <dtor@insightbb.com> wrote:
> On Monday 09 July 2007 01:29, Shem Multinymous wrote:
> > > Every input event carries a timestamp so even if there are irregularities
> > > in taking the samples you should be able to account for it.
> >
> > The issue is how good are the input event timestamps. The way it works
> > is that the EC samples the analog sensor at some fixed rate and makes
> > them available over the LPC bus. If the hdaps driver consumes these
> > samples at the same rate then the timestamps will be accurate up to a
> > small phase difference, which is mostly inconsequential. But if the
> > hdaps driver gets scheduled irregularly, its timestamps will be offset
> > by varying amounts, which will completely throw off computation (e.g.,
> > think of estimating the angular velocity).
>
> Timers do not guarantee you that they will be fired at the exact time.
> If system is under load and there are hard IRQs they will also be
> delayed.

I understand there's no guaranteed scheduling with either timers or
workqueues, but that doesn't mean we should ignore the quantitative
difference when it's so relevant.

Are there any hard numbers on this?


> > A delay of 20ms in invoking the userspace daemon is negligible, but a
> > timing variance of 20ms in the driver's measurements is devastating.
>
> Even if you know that there is such variance?

Yes, because you'll now need 4-5 samples to reasonably estimate
something like angular velocity, and 100ms are no longer negligible.


> > > Have 2nd input device's ->open() method call input_open_device() for
> > > the first one.
> >
> > Won't that create an overhead by the redundant, unused notifications?
>
> They won't leave input core so nothing really noticeable.

Sounds good, then. It's a bit of a hack, but the benefits are well
worth it (if we can resolve the scheduling issue).

  Shem

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

[Dmitry Torokhov]

On Monday 09 July 2007 02:06, Shem Multinymous wrote:
> > > > Have 2nd input device's ->open() method call input_open_device() for
> > > > the first one.
> > >
> > > Won't that create an overhead by the redundant, unused notifications?
> >
> > They won't leave input core so nothing really noticeable.
> 
> Sounds good, then. It's a bit of a hack, but the benefits are well
> worth it (if we can resolve the scheduling issue).
>

You know, I slept on it and I think I want to move the polldev into
opposite direction - to accomodte devices that need "relaxed" polling
and are ok with polling being a bit irregular and maybe even rounded
to next jiffy or something to better accomodate tickless kernels.

So I guess it would be better to drop my hdaps patch after all. 

-- 
Dmitry

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

[Shem Multinymous]

On 7/9/07, Dmitry Torokhov <dtor@insightbb.com> wrote:
> > Sounds good, then. It's a bit of a hack, but the benefits are well
> > worth it (if we can resolve the scheduling issue).
>
> You know, I slept on it and I think I want to move the polldev into
> opposite direction - to accomodte devices that need "relaxed" polling
> and are ok with polling being a bit irregular and maybe even rounded
> to next jiffy or something to better accomodate tickless kernels.

How about letting drivers and/or clients explicitly express their
scheduling needs, e.g., in terms of a desired polling rate and a
desired bound on scheduling irregularity? Given these requirements,
input-polldev will try to satisfy all requests (i.e., the harshest
one) made by the driver and current clients, on a "best effort" basis,
using minimal resources. (There should also be a way for a driver to
say "don't bother to poll more often than X or with regularity better
than Y even if a client asks for it -- the hardware isn't that fast
anyway".)

This way the same infrastructure could smoothly handle devices and
apps with vastly different polling needs.

That doesn't help much with the workqueue vs. timers issue, of course,
unless input-polldev implements *both*.

  Shem