Kernel module that shuts down the device

Kernel module that shuts down the device

[Drew Abbott]

[-- Attachment #1.1: Type: text/plain, Size: 692 bytes --]

Hi all,

I am working on a kernel module that should shut down the device when USB
is unplugged. I make a call to kernel_power_off(), but I see that it gets
stuck trying to call blocking_notifier_call_chain(&reboot_notifier_list,
(state == SYSTEM_HALT) ? SYS_HALT : SYS_POWER_OFF, NULL); in
kernel_shutdown_prepare().
There is currently another driver that has the same function, and shuts
down the device by calling kernel_power_off() successfully. I have tried
adding prints in blocking_notifier_call_chain, but then the kernel log is
flooded with calls and I can't really see what is happening.
Does anyone have any insight into why my call to kernel_power_off() is
hanging?

Thanks,
Drew

[-- Attachment #1.2: Type: text/html, Size: 800 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Kernel module that shuts down the device

[Greg KH]

On Sat, Nov 06, 2021 at 07:54:39PM -0500, Drew Abbott wrote:
> Hi all,
> 
> I am working on a kernel module that should shut down the device when USB
> is unplugged. I make a call to kernel_power_off(), but I see that it gets
> stuck trying to call blocking_notifier_call_chain(&reboot_notifier_list,
> (state == SYSTEM_HALT) ? SYS_HALT : SYS_POWER_OFF, NULL); in
> kernel_shutdown_prepare().

Where are you calling it from?  Don't call it from irq context, which is
the context that USB urbs are called from.

> There is currently another driver that has the same function, and shuts
> down the device by calling kernel_power_off() successfully. I have tried
> adding prints in blocking_notifier_call_chain, but then the kernel log is
> flooded with calls and I can't really see what is happening.
> Does anyone have any insight into why my call to kernel_power_off() is
> hanging?

Do you have a link to your driver code so it can be reviewed?

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Kernel module that shuts down the device

[Drew Abbott]

[-- Attachment #1.1: Type: text/plain, Size: 862 bytes --]

>  Where are you calling it from?  Don't call it from irq context, which is
the context that USB urbs are called from.

I am currently calling it from an irq context, in the fusb302_irq_work()
function of the in-tree fusb302.c driver. My relevant patch is below.

 > Do you have a link to your driver code so it can be reviewed?

The relevant change is here:
https://github.com/Abbotta4/linux/commit/fbfbb1db54d6bd1b10d56c9e86d08d0ecfe9abdc
You mentioned that this shouldn't be called in an irq context, but the
unplug event is detected with an irq. Where should I be calling
kernel_power_off() if not in the irq context? I think one way of doing this
would be to set a value that a heartbeat function reads in the irq, and
then the heartbeat function calls the shutdown, but this driver doesn't use
a heartbeat. Where else would I handle this?

Thank you,
Drew

[-- Attachment #1.2: Type: text/html, Size: 1117 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Kernel module that shuts down the device

[Valdis Klētnieks]

[-- Attachment #1.1: Type: text/plain, Size: 756 bytes --]

On Sun, 07 Nov 2021 18:16:55 -0600, Drew Abbott said:

> You mentioned that this shouldn't be called in an irq context, but the
> unplug event is detected with an irq. Where should I be calling
> kernel_power_off() if not in the irq context? I think one way of doing this
> would be to set a value that a heartbeat function reads in the irq, and
> then the heartbeat function calls the shutdown, but this driver doesn't use
> a heartbeat. Where else would I handle this?

There's a whole bunch of ways to schedule work in the kernel, it doesn't have to be
a heartbeat function.

Plenty of drivers are split into IRQ and non-IRQ parts (sometimes called the top and
bottom parts of the driver).  See how they get info from the IRQ part to the non-IRQ part.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 494 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Kernel module that shuts down the device

[Greg KH]

On Sun, Nov 07, 2021 at 06:16:55PM -0600, Drew Abbott wrote:
> >  Where are you calling it from?  Don't call it from irq context, which is
> the context that USB urbs are called from.
> 
> I am currently calling it from an irq context, in the fusb302_irq_work()
> function of the in-tree fusb302.c driver. My relevant patch is below.

Ah, then do not do that :)

>  > Do you have a link to your driver code so it can be reviewed?
> 
> The relevant change is here:
> https://github.com/Abbotta4/linux/commit/fbfbb1db54d6bd1b10d56c9e86d08d0ecfe9abdc
> You mentioned that this shouldn't be called in an irq context, but the
> unplug event is detected with an irq. Where should I be calling
> kernel_power_off() if not in the irq context? I think one way of doing this
> would be to set a value that a heartbeat function reads in the irq, and
> then the heartbeat function calls the shutdown, but this driver doesn't use
> a heartbeat. Where else would I handle this?

As Vladis says, there are many many ways to do this.

But step back, why would this driver ever want to shut down the machine
at all?  What problem are you trying to solve by making changes in this
driver?

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Kernel module that shuts down the device

[Drew Abbott]

[-- Attachment #1.1: Type: text/plain, Size: 1451 bytes --]

> There's a whole bunch of ways to schedule work in the kernel, it doesn't
have to be
> a heartbeat function.
>
> Plenty of drivers are split into IRQ and non-IRQ parts (sometimes called
the top and
> bottom parts of the driver).  See how they get info from the IRQ part to
the non-IRQ part.

I saw that this driver and others use workqueues to run longer functions
outside of the irq handlers in the process context, so I tried scheduling a
simple work struct that calls kernel_power_off() with this new patch:
https://github.com/Abbotta4/linux/commit/008a720d9ffc31d5b60e0ca36f2aad0a04d50f0a
after reading up on workqueues a bit here:
https://linux-kernel-labs.github.io/refs/heads/master/labs/deferred_work.html#workqueues
but the device still seems to hang at the blocking_notifier_call_chain()
call. Is there something else I am missing here, other than leaving the irq
context?

> But step back, why would this driver ever want to shut down the machine
> at all?  What problem are you trying to solve by making changes in this
> driver?

This change is to help the factory team test the devices before they are
fully assembled. There are a series of tests that they run on different
components and being able to unplug the device to trigger a shutdown is one
of their priorities. I hope that sheds some light on the context of these
patches and the strange functionality I am trying to implement. Sorry if it
caused any confusion.

Thanks,
Drew

[-- Attachment #1.2: Type: text/html, Size: 1945 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Kernel module that shuts down the device

[Greg KH]

On Mon, Nov 08, 2021 at 02:53:37PM -0600, Drew Abbott wrote:
> > There's a whole bunch of ways to schedule work in the kernel, it doesn't
> have to be
> > a heartbeat function.
> >
> > Plenty of drivers are split into IRQ and non-IRQ parts (sometimes called
> the top and
> > bottom parts of the driver).  See how they get info from the IRQ part to
> the non-IRQ part.
> 
> I saw that this driver and others use workqueues to run longer functions
> outside of the irq handlers in the process context, so I tried scheduling a
> simple work struct that calls kernel_power_off() with this new patch:
> https://github.com/Abbotta4/linux/commit/008a720d9ffc31d5b60e0ca36f2aad0a04d50f0a
> after reading up on workqueues a bit here:
> https://linux-kernel-labs.github.io/refs/heads/master/labs/deferred_work.html#workqueues
> but the device still seems to hang at the blocking_notifier_call_chain()
> call. Is there something else I am missing here, other than leaving the irq
> context?

I do not know, I'm not going to dig around on random web sites for
patches, sorry :)

> > But step back, why would this driver ever want to shut down the machine
> > at all?  What problem are you trying to solve by making changes in this
> > driver?
> 
> This change is to help the factory team test the devices before they are
> fully assembled. There are a series of tests that they run on different
> components and being able to unplug the device to trigger a shutdown is one
> of their priorities. I hope that sheds some light on the context of these
> patches and the strange functionality I am trying to implement. Sorry if it
> caused any confusion.

Why are you trying to do this in the typec driver?  You should be able
to do all of this in userspace, just use libusb to get a list of devices
connected and when one is removed, reboot the machine.  There should not
be any need to write kernel code at all here.

Or, if you really want to write kernel code, do so for when you remove
the actual USB device itself, not in the typec code, so write a USB
driver for the USB device, not controller.

thanks,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies