Kernel module that shuts down the device
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
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
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/fbfbb1db54d6bd1b10d56c9e86d08d0ecfe... 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
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.
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/fbfbb1db54d6bd1b10d56c9e86d08d0ecfe... 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
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/008a720d9ffc31d5b60e0ca36f2aad0a04d... after reading up on workqueues a bit here: https://linux-kernel-labs.github.io/refs/heads/master/labs/deferred_work.htm... 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
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/008a720d9ffc31d5b60e0ca36f2aad0a04d... after reading up on workqueues a bit here: https://linux-kernel-labs.github.io/refs/heads/master/labs/deferred_work.htm... 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
participants (3)
-
Drew Abbott -
Greg KH -
Valdis Klētnieks