Re: Re: Re: Re: Re: Re: [PATCH 0/2] Power management fixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Dieter,

On Tue, Feb 06, 2024 at 04:06:36PM +0100, Dieter Mummenschanz wrote:
> Hello Niklas,
> 
> > That is a good suggestion.
> > However, I'm hoping that this series:
> > https://lore.kernel.org/linux-ide/3e50681d-7a68-4c4a-91f6-278a3cfe23f8@xxxxxxx/T/[https://lore.kernel.org/linux-> ide/3e50681d-7a68-4c4a-91f6-278a3cfe23f8@xxxxxxx/T/]
> 
> > Will avoid the need for that, as it would kill the
> > "board_ahci_low_power" board type completely.
> 
> Very nice. If there is anything "baked" enough to test, please let me know.

It should be ready for testing:
https://github.com/floatious/linux/tree/external-port-v2

With this, you should no longer need:
for foo in /sys/class/scsi_host/host*/link_power_management_policy;
  do echo med_power_with_dipm > $foo;
done

(Assuming you have compiled your kernel with CONFIG_SATA_MOBILE_LPM_POLICY=3).


> 
> > After talking with Damien, we are still completely lost.
> > commit fd3a6837d8e1 ("ata: libata-core: Fix ata_pci_shutdown_one()")
> > only modifies the ata_pci_shutdown_one(), which is only called on shutdown.
> 
> > It is not called in the system suspend / system resume path,
> > so it should not be possible for this commit to have an impact.
> 
> I don't understand it either. For what it's worth, this morning after resuming, my system was stuck at pc2 for 10+ Minutes. After issuing hdparm -Y /dev/sdb I saw lower transitions down to pc8 right away without(!) issuing the same command for sda. So maybe fd3a6837d8e1 isn't the culprit after all. I'll keep testing.

You shouldn't need to do:
$ hdparm -Y

As that bypasses some of the internal logic in libata.

I assume that you didn't need this on v6.6 and older?

(Instead libata should put the device to standby or sleep itself,
it shouldn't need to be done explicitly by the user.)


> 
> 
> > Reading all the emails and bugzilla again, what you are seeing is that:
> 
> > On a fresh boot, and leaving the computer untouched for a few minutes,
> > and looking at e.g. turbostat, some of your CPUs have spent time in
> > the lower-power C-states, e.g. C8/C9/C10.
> 
> Correct! Btw. I'm looking at powertop and also have a daemon that feeds the current c-states to conky for display so I have an eye on the c-states at all times.
> 
> > However, if you do a system suspend + system resume, and then leave
> > the computer untouched for a few minutes, and look at e.g. turbotstat,
> > _none_ of your CPUs have spend time in the lower-power C-states.
> 
> > Is this correct?
> 
> Yes! The system spends ~90% in pc2 no lower c-states.
> 
> > If so, it seems that suspend/resume must have messed up some setting...
> 
> > The commit that added the code to ata_pci_shutdown_one()
> > was added in:
> > 5b6fba546da2 ("ata: libata-core: Detach a port devices on shutdown")
> > This was first added in v6.7-rc1.
> > (So this commit was never included in v6.6.)
> 
> > The code added in 5b6fba546da2 was later removed in:
> > commit fd3a6837d8e1 ("ata: libata-core: Fix ata_pci_shutdown_one()")
> > which was first added in v6.7-rc1.
> 
> > Are you certain that v6.6 works and v6.7 is bad?
> 
> Yes, absolutely!

How certain are you that v6.6 works?

Could it be the same problem there, that it works sometimes and doesn't
work sometimes?

The reason why I'm asking is that Damien's major changes got included
in v6.6-rc4:
https://lore.kernel.org/linux-ide/20230929133324.164211-1-dlemoal@xxxxxxxxxx/

So I would be less surprised if you said that you can enter pc8 on every
boot on v6.5, but on v6.6 it only works occasionally.

But if you can enter pc8 on every boot on v6.6, but not on v6.7,
then it is probably easier to figure out which commit that broke
things, as there were not that many suspend/resume related changes
added in v6.7.


Kind regards,
Niklas




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux