Re: Why hid-sensor-hub's IIO doesn't work properly in >= 4.3 (possibly badly bisected)

Hey Rafael,

On Fri, 2017-01-27 at 00:07 +0100, Rafael J. Wysocki wrote:

On Thu, Jan 26, 2017 at 4:15 PM, Bastien Nocera [off-list ref]
wrote:

quoted

On Fri, 2017-01-20 at 16:52 +0100, Bastien Nocera wrote:

quoted

Hey,

TLDR:
# first bad commit: [50ba22479c324c0d9dc8134d519dcba92d83a8a7]
Merge
back earlier ACPI PM material for v4.3.

hid-sensor-hub devices only start sending events through the IIO
trigger after a suspend/resume cycle.

Other IIO drivers don't seem to exhibit those problems.

I had another go, trying to break into the merge commit, and it's
slightly more precise, but still doesn't make any sense to me.

# first bad commit: [3431e490b50356b56084305a2e93b3a980802b22]
Merge
branch 'acpi-scan' into acpi-pm

Any ideas? I'm running out of them...

The parents of this merge are 5af310a8ee70 and 1dcc3d3362b0.

Can you please check if the problem is present in any of them alone?

Both of those were already in the git bisect output:
# good: [5af310a8ee70dd6a588c8ee1d4487a230a7b7b65] Merge tag 'ib-mfd-base-acpi-dma-v4.3' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd into acpi-pm
git bisect good 5af310a8ee70dd6a588c8ee1d4487a230a7b7b65

# good: [1dcc3d3362b0c97e48290f7786be85b4cec2a147] ACPI / bus: Move ACPI bus type registration
git bisect good 1dcc3d3362b0c97e48290f7786be85b4cec2a147

Or did I do this wrong?

If not, it is theoretically possible that the merge itself had
introduced it (as this is not a trivial merge).

Yeah, I tried diving into the merge, but never managed to do so.

Cheers

+ Srinivas

On Tuesday, January 31, 2017 02:37:43 PM Bastien Nocera wrote:

Hey Rafael,

On Fri, 2017-01-27 at 00:07 +0100, Rafael J. Wysocki wrote:

quoted

On Thu, Jan 26, 2017 at 4:15 PM, Bastien Nocera [off-list ref]
wrote:

quoted

On Fri, 2017-01-20 at 16:52 +0100, Bastien Nocera wrote:

quoted

Hey,

TLDR:
# first bad commit: [50ba22479c324c0d9dc8134d519dcba92d83a8a7]
Merge
back earlier ACPI PM material for v4.3.

hid-sensor-hub devices only start sending events through the IIO
trigger after a suspend/resume cycle.

Other IIO drivers don't seem to exhibit those problems.

I had another go, trying to break into the merge commit, and it's
slightly more precise, but still doesn't make any sense to me.

# first bad commit: [3431e490b50356b56084305a2e93b3a980802b22]
Merge
branch 'acpi-scan' into acpi-pm

Any ideas? I'm running out of them...

The parents of this merge are 5af310a8ee70 and 1dcc3d3362b0.

Can you please check if the problem is present in any of them alone?

Both of those were already in the git bisect output:
# good: [5af310a8ee70dd6a588c8ee1d4487a230a7b7b65] Merge tag 'ib-mfd-base-acpi-dma-v4.3' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd into acpi-pm
git bisect good 5af310a8ee70dd6a588c8ee1d4487a230a7b7b65

# good: [1dcc3d3362b0c97e48290f7786be85b4cec2a147] ACPI / bus: Move ACPI bus type registration
git bisect good 1dcc3d3362b0c97e48290f7786be85b4cec2a147

Or did I do this wrong?

quoted

If not, it is theoretically possible that the merge itself had
introduced it (as this is not a trivial merge).

Yeah, I tried diving into the merge, but never managed to do so.

Srinivas, does it sound like anything familiar to you?

Thanks,
Rafael

On Thu, 2017-02-02 at 00:38 +0100, Rafael J. Wysocki wrote:

+ Srinivas

On Tuesday, January 31, 2017 02:37:43 PM Bastien Nocera wrote:

quoted

Hey Rafael,

On Fri, 2017-01-27 at 00:07 +0100, Rafael J. Wysocki wrote:

quoted

On Thu, Jan 26, 2017 at 4:15 PM, Bastien Nocera <hadess@hadess.ne
t>
wrote:

quoted

On Fri, 2017-01-20 at 16:52 +0100, Bastien Nocera wrote:

quoted

Hey,

TLDR:
# first bad commit:
[50ba22479c324c0d9dc8134d519dcba92d83a8a7]
Merge
back earlier ACPI PM material for v4.3.

hid-sensor-hub devices only start sending events through the
IIO
trigger after a suspend/resume cycle.

Srinivas, does it sound like anything familiar to you?

I guess this is related to
https://github.com/hadess/iio-sensor-proxy/issues/82

There is some race between user space and iio. So the driver powerup
never gets called back to power a hub during system boot. So the
workaround was to add to systemd unit file for iio-sensor-proxy 
[Unit]
After=multi-user.target

Something changed timings in the kernel, which triggered this issue.
I never got chance to root cause this.

Thanks,
Srinivas

On Wed, 2017-02-01 at 15:57 -0800, Srinivas Pandruvada wrote:

On Thu, 2017-02-02 at 00:38 +0100, Rafael J. Wysocki wrote:

quoted

+ Srinivas

On Tuesday, January 31, 2017 02:37:43 PM Bastien Nocera wrote:

quoted

Hey Rafael,

On Fri, 2017-01-27 at 00:07 +0100, Rafael J. Wysocki wrote:

quoted

On Thu, Jan 26, 2017 at 4:15 PM, Bastien Nocera <hadess@hadess.
ne
t>
wrote:

quoted

On Fri, 2017-01-20 at 16:52 +0100, Bastien Nocera wrote:

quoted

Hey,

TLDR:
# first bad commit:
[50ba22479c324c0d9dc8134d519dcba92d83a8a7]
Merge
back earlier ACPI PM material for v4.3.

hid-sensor-hub devices only start sending events through
the
IIO
trigger after a suspend/resume cycle.

quoted

Srinivas, does it sound like anything familiar to you?

I guess this is related to
https://github.com/hadess/iio-sensor-proxy/issues/82

There is some race between user space and iio. So the driver powerup
never gets called back to power a hub during system boot. So the
workaround was to add to systemd unit file for iio-sensor-proxy 
[Unit]
After=multi-user.target

Something changed timings in the kernel, which triggered this issue.

I don't think it's simply "timings", or at least it's a big enough
window of opportunity that I can reproduce the bug 100% of the time
when not adding timeouts to iio-sensor-proxy's timeout.

Putting the machine on suspend and resuming it also fixes the problem
(for a machine I've been testing that can be suspended, it's not an
option for all of them).

I never got chance to root cause this.

Well, at least the root cause is limited to a single commit, shame it's
a merge one.

On Thursday, February 02, 2017 11:35:05 AM Bastien Nocera wrote:

On Wed, 2017-02-01 at 15:57 -0800, Srinivas Pandruvada wrote:

quoted

On Thu, 2017-02-02 at 00:38 +0100, Rafael J. Wysocki wrote:

quoted

+ Srinivas

On Tuesday, January 31, 2017 02:37:43 PM Bastien Nocera wrote:

quoted

Hey Rafael,

On Fri, 2017-01-27 at 00:07 +0100, Rafael J. Wysocki wrote:

quoted

On Thu, Jan 26, 2017 at 4:15 PM, Bastien Nocera <hadess@hadess.
ne
t>
wrote:

quoted

On Fri, 2017-01-20 at 16:52 +0100, Bastien Nocera wrote:

quoted

Hey,

TLDR:
# first bad commit:
[50ba22479c324c0d9dc8134d519dcba92d83a8a7]
Merge
back earlier ACPI PM material for v4.3.

hid-sensor-hub devices only start sending events through
the
IIO
trigger after a suspend/resume cycle.

quoted

Srinivas, does it sound like anything familiar to you?

I guess this is related to
https://github.com/hadess/iio-sensor-proxy/issues/82

There is some race between user space and iio. So the driver powerup
never gets called back to power a hub during system boot. So the
workaround was to add to systemd unit file for iio-sensor-proxy 
[Unit]
After=multi-user.target

Something changed timings in the kernel, which triggered this issue.

I don't think it's simply "timings", or at least it's a big enough
window of opportunity that I can reproduce the bug 100% of the time
when not adding timeouts to iio-sensor-proxy's timeout.

Putting the machine on suspend and resuming it also fixes the problem
(for a machine I've been testing that can be suspended, it's not an
option for all of them).

quoted

I never got chance to root cause this.

Well, at least the root cause is limited to a single commit, shame it's
a merge one.

In fact this is a merge that doesn't change any code by itself (I thought it
did, but that was not correct), so if that had been more than timings, you'd
have seen breakage on at least one of the merged branches.

Moreover, it merges the commits under 3431e490b503 back on top of material
that went into 4.2, so if you only see the problem in 4.3 and later, this has to
be the 3431e490b503 branch.

Can you double check 3431e490b503 alone, please?

Thanks,
Rafael

On Thu, 2017-02-02 at 12:33 +0100, Rafael J. Wysocki wrote:

On Thursday, February 02, 2017 11:35:05 AM Bastien Nocera wrote:

quoted

On Wed, 2017-02-01 at 15:57 -0800, Srinivas Pandruvada wrote:

quoted

On Thu, 2017-02-02 at 00:38 +0100, Rafael J. Wysocki wrote:

quoted

+ Srinivas

On Tuesday, January 31, 2017 02:37:43 PM Bastien Nocera wrote:

quoted

Hey Rafael,

On Fri, 2017-01-27 at 00:07 +0100, Rafael J. Wysocki wrote:

quoted

On Thu, Jan 26, 2017 at 4:15 PM, Bastien Nocera <hadess@had
ess.
ne
t>
wrote:

quoted

On Fri, 2017-01-20 at 16:52 +0100, Bastien Nocera wrote:

quoted

Hey,

TLDR:
# first bad commit:
[50ba22479c324c0d9dc8134d519dcba92d83a8a7]
Merge
back earlier ACPI PM material for v4.3.

hid-sensor-hub devices only start sending events
through
the
IIO
trigger after a suspend/resume cycle.

quoted

Srinivas, does it sound like anything familiar to you?

I guess this is related to
https://github.com/hadess/iio-sensor-proxy/issues/82

There is some race between user space and iio. So the driver
powerup
never gets called back to power a hub during system boot. So the
workaround was to add to systemd unit file for iio-sensor-proxy 
[Unit]
After=multi-user.target

Something changed timings in the kernel, which triggered this
issue.

I don't think it's simply "timings", or at least it's a big enough
window of opportunity that I can reproduce the bug 100% of the time
when not adding timeouts to iio-sensor-proxy's timeout.

Putting the machine on suspend and resuming it also fixes the
problem
(for a machine I've been testing that can be suspended, it's not an
option for all of them).

quoted

I never got chance to root cause this.

Well, at least the root cause is limited to a single commit, shame
it's
a merge one.

In fact this is a merge that doesn't change any code by itself (I
thought it
did, but that was not correct), so if that had been more than
timings, you'd
have seen breakage on at least one of the merged branches.

Moreover, it merges the commits under 3431e490b503 back on top of
material
that went into 4.2, so if you only see the problem in 4.3 and later,
this has to
be the 3431e490b503 branch.

Can you double check 3431e490b503 alone, please?

I don't understand what you're asking of me. I'm not a git master, and
I've never had to deal with merge commits.

Did you want me to run "git reset --hard 3431e490b503" and test the
resulting kernel? I'm pretty sure that's equivalent what my 3 runs of
bisection have done, and it failed.

Cheers

On Thu, Feb 2, 2017 at 1:42 PM, Bastien Nocera [off-list ref] wrote:

On Thu, 2017-02-02 at 12:33 +0100, Rafael J. Wysocki wrote:

quoted

On Thursday, February 02, 2017 11:35:05 AM Bastien Nocera wrote:

quoted

On Wed, 2017-02-01 at 15:57 -0800, Srinivas Pandruvada wrote:

quoted

On Thu, 2017-02-02 at 00:38 +0100, Rafael J. Wysocki wrote:

quoted

+ Srinivas

On Tuesday, January 31, 2017 02:37:43 PM Bastien Nocera wrote:

quoted

Hey Rafael,

On Fri, 2017-01-27 at 00:07 +0100, Rafael J. Wysocki wrote:

quoted

On Thu, Jan 26, 2017 at 4:15 PM, Bastien Nocera <hadess@had
ess.
ne
t>
wrote:

quoted

On Fri, 2017-01-20 at 16:52 +0100, Bastien Nocera wrote:

quoted

Hey,

TLDR:
# first bad commit:
[50ba22479c324c0d9dc8134d519dcba92d83a8a7]
Merge
back earlier ACPI PM material for v4.3.

hid-sensor-hub devices only start sending events
through
the
IIO
trigger after a suspend/resume cycle.

quoted

Srinivas, does it sound like anything familiar to you?

I guess this is related to
https://github.com/hadess/iio-sensor-proxy/issues/82

There is some race between user space and iio. So the driver
powerup
never gets called back to power a hub during system boot. So the
workaround was to add to systemd unit file for iio-sensor-proxy
[Unit]
After=multi-user.target

Something changed timings in the kernel, which triggered this
issue.

I don't think it's simply "timings", or at least it's a big enough
window of opportunity that I can reproduce the bug 100% of the time
when not adding timeouts to iio-sensor-proxy's timeout.

Putting the machine on suspend and resuming it also fixes the
problem
(for a machine I've been testing that can be suspended, it's not an
option for all of them).

quoted

I never got chance to root cause this.

Well, at least the root cause is limited to a single commit, shame
it's
a merge one.

In fact this is a merge that doesn't change any code by itself (I
thought it
did, but that was not correct), so if that had been more than
timings, you'd
have seen breakage on at least one of the merged branches.

Moreover, it merges the commits under 3431e490b503 back on top of
material
that went into 4.2, so if you only see the problem in 4.3 and later,
this has to
be the 3431e490b503 branch.

Can you double check 3431e490b503 alone, please?

I don't understand what you're asking of me. I'm not a git master, and
I've never had to deal with merge commits.

Did you want me to run "git reset --hard 3431e490b503" and test the
resulting kernel? I'm pretty sure that's equivalent what my 3 runs of
bisection have done, and it failed.

I must have misunderstood you, sorry.

This means that the error is present in 3431e490b503, though, so
something on this branch had introduced it, most likely.

Thanks,
Rafael