mbox series

[v2,0/2] Abrupt Shutdown for NVMe SSD

Message ID 20210726132223.1661-1-sshivamurthy@micron.com
Headers show
Series Abrupt Shutdown for NVMe SSD | expand

Message

shiva.linuxworks at gmail.com July 26, 2021, 1:22 p.m. UTC
From: Shivamurthy Shastri <sshivamurthy@micron.com>

In the platform with a limited backup power supply, devices like NVMe
SSD does unsafe shutdown.

These two patches address this issue by adding a power loss imminent
flag. The platform will enable the power loss imminent flag if the
platform's power is running on the limited backup power supply. During
the shutdown, the driver checks this information and pwerforms the
abrupt shutdown.

Shivamurthy Shastri (2):
  PM: introduce power loss imminent
  nvme: Add abrupt shutdown support

 drivers/nvme/host/core.c |  7 ++++++-
 drivers/nvme/host/pci.c  |  6 ++++--
 include/linux/suspend.h  | 28 +++++++++++++++++++++++++---
 3 files changed, 35 insertions(+), 6 deletions(-)

Comments

Pavel Machek July 26, 2021, 7:41 p.m. UTC | #1
Hi!

> If the shutdown is pwerformed when the platform is running on the
> limited backup power supply, some of the devices might not have enough
> power to perform a clean shutdown.
> 
> It is necessary to inform the driver about the limited backup power
> supply, to allow the driver to decide to perform the minimal required
> operation for a fast and clean shutdown.

If you can do shutdown that is fast & clean, why not do it always?

How fast is normal shutdown vs. fast shutdown?

> +#define PM_SUSPEND_FLAG_POWER_LOSS_IMMINENT	BIT(3)

I believe we should be more concrete here. Like explaining use (did
UPS say battery is low? Or does it mean 10 seconds remaining? Or...?)

Plus, who sets this flag? Userland?

Best regards,
								Pavel


--
Keith Busch July 26, 2021, 8:16 p.m. UTC | #2
On Mon, Jul 26, 2021 at 09:41:46PM +0200, Pavel Machek wrote:
> > If the shutdown is pwerformed when the platform is running on the
> > limited backup power supply, some of the devices might not have enough
> > power to perform a clean shutdown.
> > 
> > It is necessary to inform the driver about the limited backup power
> > supply, to allow the driver to decide to perform the minimal required
> > operation for a fast and clean shutdown.
> 
> If you can do shutdown that is fast & clean, why not do it always?

At least for nvme, I don't think this faster shutdown qualifies as
"clean". It's just a little better than removing power without telling
the device. Typical implementations take longer to become ready on their
next power-on following the abrupt shutdown sequence.
Shivamurthy Shastri (sshivamurthy) July 29, 2021, 10:37 a.m. UTC | #3
Micron Confidential

Hi,

> 

> 

> On Mon, Jul 26, 2021 at 01:22:21PM +0000, shiva.linuxworks@gmail.com wrote:

> > In the platform with a limited backup power supply, devices like NVMe

> > SSD does unsafe shutdown.

> >

> > These two patches address this issue by adding a power loss imminent

> > flag. The platform will enable the power loss imminent flag if the

> > platform's power is running on the limited backup power supply. During

> > the shutdown, the driver checks this information and pwerforms the

> > abrupt shutdown.

> 

> I think the pm framework and nvme usage are ok, but you need a platform

> specific caller to set the new power loss flag before this should be

> considered, otherwise this is just unreachable code.


Sure, I will send platform specific caller along with V3.

Thanks,
Shiva

Micron Confidential