mbox series

[0/3] staging: vchiq: use interruptible waits

Message ID 20190405113423.14495-1-nsaenzjulienne@suse.de (mailing list archive)
Headers show
Series staging: vchiq: use interruptible waits | expand

Message

Nicolas Saenz Julienne April 5, 2019, 11:34 a.m. UTC
Hi,
this series tries to address an issue that came up in Raspbian's kernel
tree [1]. After pulling from upstream some changes that moved wait calls
from a custom implementation to the more standard killable family some
users complained that all the VCHIQ threads showed up in D state (which
is the expected behaviour).

The custom implementation we deleted tried to mimic the killable family
of functions, yet accepted more signals than the later.  SIGKILL |
SIGINT | SIGQUIT | SIGTRAP | SIGSTOP | SIGCONT for the custom
implementation as opposed to plain old SIGKILL.

Raspbian maintainers decided roll back some of those changes and leave
the wait functions as interruptible. Hence creating some divergence
between both trees.

One could argue that not liking having the threads stuck in D state is
not really a software issue. It's more a cosmetic thing that can scare
people when they look at "uptime". On the other hand, if we are ever to
unstage this driver, we'd really need a proper justification for using
the killable family of functions. Which I think it's not really clear at
the moment.

As Raspbian's kernel has been working for a while with interruptible
waits I propose we follow through. If needed we can always go back to
killable. But at least we'll have a proper understanding on the actual
needs. In the end the driver is in staging, and the potential for errors
small.

Regards,
Nicolas

[1] https://github.com/raspberrypi/linux/issues/2881

---

Nicolas Saenz Julienne (3):
  Revert "staging: vchiq_2835_arm: quit using custom
    down_interruptible()"
  Revert "staging: vchiq: switch to wait_for_completion_killable"
  staging: vchiq: make wait events interruptible

 .../interface/vchiq_arm/vchiq_2835_arm.c      |  2 +-
 .../interface/vchiq_arm/vchiq_arm.c           | 21 +++++++++--------
 .../interface/vchiq_arm/vchiq_core.c          | 23 ++++++++++---------
 .../interface/vchiq_arm/vchiq_util.c          |  6 ++---
 4 files changed, 27 insertions(+), 25 deletions(-)

Comments

Stefan Wahren April 5, 2019, 12:40 p.m. UTC | #1
Hi Nicolas,

Am 05.04.19 um 13:34 schrieb Nicolas Saenz Julienne:
> Hi,
> this series tries to address an issue that came up in Raspbian's kernel
> tree [1]. After pulling from upstream some changes that moved wait calls
> from a custom implementation to the more standard killable family some
> users complained that all the VCHIQ threads showed up in D state (which
> is the expected behaviour).

this issue has already been noticed in mainline distributions [1],[2].

> The custom implementation we deleted tried to mimic the killable family
> of functions, yet accepted more signals than the later.  SIGKILL |
> SIGINT | SIGQUIT | SIGTRAP | SIGSTOP | SIGCONT for the custom
> implementation as opposed to plain old SIGKILL.
>
> Raspbian maintainers decided roll back some of those changes and leave
> the wait functions as interruptible. Hence creating some divergence
> between both trees.
>
> One could argue that not liking having the threads stuck in D state is
> not really a software issue. It's more a cosmetic thing that can scare
> people when they look at "uptime". On the other hand, if we are ever to
> unstage this driver, we'd really need a proper justification for using
> the killable family of functions. Which I think it's not really clear at
> the moment.

I like to see this decision as a short comment in the code to prevent
other for doing this mistake again.

Thanks

Stefan

>
> As Raspbian's kernel has been working for a while with interruptible
> waits I propose we follow through. If needed we can always go back to
> killable. But at least we'll have a proper understanding on the actual
> needs. In the end the driver is in staging, and the potential for errors
> small.
>
> Regards,
> Nicolas
>
> [1] https://github.com/raspberrypi/linux/issues/2881
>
[1] - https://archlinuxarm.org/forum/viewtopic.php?f=65&t=13485

[2] -
https://lists.fedoraproject.org/archives/list/arm@lists.fedoraproject.org/message/GBXGJ7DOV5CQQXFPOZCXTRD6W4BEPT4Q/