Re: [PATCH 0/5] nvme APST fixes/improvements for 4.11
On Wed, Apr 19, 2017 at 10:19 PM, Christoph Hellwig wrote: > On Wed, Apr 19, 2017 at 09:52:17PM -0700, Andy Lutomirski wrote: >> > I can make it so that force_apst=0 means no APST and force_apst=1 mean >> > yes APST and we could try again with a quirk list for 4.12. There's a >> > decent chance that a few more weeks with Ubuntu having APST on will >> > shake out all the problems fairly quickly. >> >> Here's a more concrete and more sensible proposal: > > Can we just have force_apst=on to force it on, force_apst=off to turn > it off, and leave it with that? And yes, I mean the strings instead > of the weird numbers. That's essentially what the patch I emailed here does -- it's a regular bool param, so force_apst=1, force_apst=Y, and force_apst=y all mean the same thing. I can just leave it like that.
Re: [PATCH 0/5] nvme APST fixes/improvements for 4.11
On Wed, Apr 19, 2017 at 09:52:17PM -0700, Andy Lutomirski wrote: > > I can make it so that force_apst=0 means no APST and force_apst=1 mean > > yes APST and we could try again with a quirk list for 4.12. There's a > > decent chance that a few more weeks with Ubuntu having APST on will > > shake out all the problems fairly quickly. > > Here's a more concrete and more sensible proposal: Can we just have force_apst=on to force it on, force_apst=off to turn it off, and leave it with that? And yes, I mean the strings instead of the weird numbers.
Re: [PATCH 0/5] nvme APST fixes/improvements for 4.11
On Wed, Apr 19, 2017 at 8:55 PM, Andy Lutomirski wrote: > On Wed, Apr 19, 2017 at 8:10 PM, Jens Axboe wrote: >> On Wed, Apr 19 2017, Andy Lutomirski wrote: >>> Sorry for waiting so long for this. I was waiting for feedback from >>> Samsung, but they haven't root-caused the issue yet, and I should >>> have just done this from the beginning. >>> >>> This series makes APST more debuggable and updates the quirk list. >>> The quirks I'm aware of are: >>> >>> - Samsung 950 series SSDs in Dell XPS 15 9550 and Precision 5510 >>>laptops (which are essentially the same laptop) can lose their >>>PCIe link if they're allowed to use the deepest APST state. >>>Samsung engineers have an affected system and are working on >>>it. The same exact SSDs in other machines (even an XPS 13) >>>seem to work fine. >>> >>> - One Toshiba device malfunctions if APST is used at all. >> >> You need to split this series in two, patches 1-3 can wait. For 4.11, >> all we need to do is turn off APST on any device that potentially has >> this problem. >> >>> One thing that improves my confidence that there aren't too many >>> more problems with APST is that Ubuntu has backported APST to Zesty, >>> so it's already gotten a bit of testing in a widely used (if very >>> new) release. >> >> Honestly, I think the best path for 4.11 is to turn off APST by default, >> make it opt-in instead. I don't share your optimism here, as I made >> clear back from before we even merged this feature. >> >> > > I can make it so that force_apst=0 means no APST and force_apst=1 mean > yes APST and we could try again with a quirk list for 4.12. There's a > decent chance that a few more weeks with Ubuntu having APST on will > shake out all the problems fairly quickly. Here's a more concrete and more sensible proposal: For 4.11: force_apst=0: Default. APST off on all Samsung 950-like devices regardless of what laptop and on the Toshiba device. force_apst=1: Use APST except where known bad. APST deepest state disabled on Samsung 950-like devices on XPS 15 and Precision 5510. APST off on the Toshiba device. force_apst=2: APST fully on regardless of any quirks. For 4.12-rc1: force_apst=0 works like force_apst=1, but we keep both values for compatibility and in case we need to add another overly broad quirk some day. Would something like this make sense?
Re: [PATCH 0/5] nvme APST fixes/improvements for 4.11
On Wed, Apr 19, 2017 at 8:10 PM, Jens Axboe wrote: > On Wed, Apr 19 2017, Andy Lutomirski wrote: >> Sorry for waiting so long for this. I was waiting for feedback from >> Samsung, but they haven't root-caused the issue yet, and I should >> have just done this from the beginning. >> >> This series makes APST more debuggable and updates the quirk list. >> The quirks I'm aware of are: >> >> - Samsung 950 series SSDs in Dell XPS 15 9550 and Precision 5510 >>laptops (which are essentially the same laptop) can lose their >>PCIe link if they're allowed to use the deepest APST state. >>Samsung engineers have an affected system and are working on >>it. The same exact SSDs in other machines (even an XPS 13) >>seem to work fine. >> >> - One Toshiba device malfunctions if APST is used at all. > > You need to split this series in two, patches 1-3 can wait. For 4.11, > all we need to do is turn off APST on any device that potentially has > this problem. > >> One thing that improves my confidence that there aren't too many >> more problems with APST is that Ubuntu has backported APST to Zesty, >> so it's already gotten a bit of testing in a widely used (if very >> new) release. > > Honestly, I think the best path for 4.11 is to turn off APST by default, > make it opt-in instead. I don't share your optimism here, as I made > clear back from before we even merged this feature. > > I can make it so that force_apst=0 means no APST and force_apst=1 mean yes APST and we could try again with a quirk list for 4.12. There's a decent chance that a few more weeks with Ubuntu having APST on will shake out all the problems fairly quickly. --Andy
Re: [PATCH 0/5] nvme APST fixes/improvements for 4.11
On Wed, Apr 19 2017, Andy Lutomirski wrote: > Sorry for waiting so long for this. I was waiting for feedback from > Samsung, but they haven't root-caused the issue yet, and I should > have just done this from the beginning. > > This series makes APST more debuggable and updates the quirk list. > The quirks I'm aware of are: > > - Samsung 950 series SSDs in Dell XPS 15 9550 and Precision 5510 >laptops (which are essentially the same laptop) can lose their >PCIe link if they're allowed to use the deepest APST state. >Samsung engineers have an affected system and are working on >it. The same exact SSDs in other machines (even an XPS 13) >seem to work fine. > > - One Toshiba device malfunctions if APST is used at all. You need to split this series in two, patches 1-3 can wait. For 4.11, all we need to do is turn off APST on any device that potentially has this problem. > One thing that improves my confidence that there aren't too many > more problems with APST is that Ubuntu has backported APST to Zesty, > so it's already gotten a bit of testing in a widely used (if very > new) release. Honestly, I think the best path for 4.11 is to turn off APST by default, make it opt-in instead. I don't share your optimism here, as I made clear back from before we even merged this feature. -- Jens Axboe
[PATCH 0/5] nvme APST fixes/improvements for 4.11
Sorry for waiting so long for this. I was waiting for feedback from Samsung, but they haven't root-caused the issue yet, and I should have just done this from the beginning. This series makes APST more debuggable and updates the quirk list. The quirks I'm aware of are: - Samsung 950 series SSDs in Dell XPS 15 9550 and Precision 5510 laptops (which are essentially the same laptop) can lose their PCIe link if they're allowed to use the deepest APST state. Samsung engineers have an affected system and are working on it. The same exact SSDs in other machines (even an XPS 13) seem to work fine. - One Toshiba device malfunctions if APST is used at all. One thing that improves my confidence that there aren't too many more problems with APST is that Ubuntu has backported APST to Zesty, so it's already gotten a bit of testing in a widely used (if very new) release. Andy Lutomirski (5): nvme: Fix APST comment nvme: Display raw APST configuration via DYNAMIC_DEBUG nvme: Add nvme_core.force_apst to ignore the NO_APST quirk nvme: Adjust the Samsung APST quirk nvme: Quirk APST off on "THNSF5256GPUK TOSHIBA" drivers/nvme/host/core.c | 61 drivers/nvme/host/nvme.h | 5 drivers/nvme/host/pci.c | 26 - 3 files changed, 81 insertions(+), 11 deletions(-) -- 2.9.3
