Re: [PATCH v9 0/4] shut down devices asynchronously

From: stuart hayes
Date: Fri Oct 18 2024 - 20:27:16 EST


On 10/18/2024 4:37 AM, Greg Kroah-Hartman wrote:
On Fri, Oct 18, 2024 at 11:14:51AM +0200, Lukas Wunner wrote:
On Fri, Oct 18, 2024 at 07:49:51AM +0200, Greg Kroah-Hartman wrote:
On Fri, Oct 18, 2024 at 03:26:05AM +0000, Michael Kelley wrote:
In the process, the workqueue code spins up additional worker threads
to handle the load. On the Hyper-V VM, 210 to 230 new kernel
threads are created during device_shutdown(), depending on the
timing. On the Pi 5, 253 are created. The max for this workqueue is
WQ_DFL_ACTIVE (256).
[...]
I don't think we can put this type of load on all systems just to handle
one specific type of "bad" hardware that takes long periods of time to
shutdown, sorry.

Parallelizing shutdown means shorter reboot times, less downtime,
less cost for CSPs.

For some systems, yes, but as have been seen here, it comes at the
offset of a huge CPU load at shutdown, with sometimes longer reboot
times.

Modern servers (e.g. Sierra Forest with 288 cores) should handle
this load easily and may see significant benefits from parallelization.

"may see", can you test this?

Perhaps a solution is to cap async shutdown based on the number of cores,
but always use async for certain device classes (e.g. nvme_subsys_class)?

Maybe, but as-is, we can't take the changes this way, sorry. That is a
regression from the situation of working hardware that many people have.

thanks,

greg k-h

Thank you both for your time and effort considering this. It didn't occur to me that an extra few 10s of milliseconds (or maxing out the async workqueue) would be an issue.

To answer your earlier question (Michael), there shouldn't be a possibility of deadlock regardless of the number of devices. While the device shutdowns are scheduled on a workqueue rather than run in a loop, they are still scheduled in the same order as they are without this patch, any any device that is scheduled for shutdown should never have to wait for device that hasn't yet been scheduled. So even if only one device shutdown could be scheduled at a time, it should still work without deadlocking--it just wouldn't be able to do shutdowns in parallel.

And I believe there is still a benefit to having async shutdown enabled even with one core. The NVMe shutdowns that take a while involve waiting for drives to finish commands, so they are mostly just sleeping. Workqueues will schedule another worker if one worker sleeps, so even a single core system should be able to get a number of NVMe drives started on their shutdowns in parallel.

I'll see what I can to do limit the amount of stuff that gets put on the
workqueue, though. I can likely limit it to just the asynchronous device shutdowns (NVMe shutdowns), plus any devices that have to wait for them (i.e., any devices of which they are dependents or consumers).