From: Damien Le Moal <dlemoal@kernel.org>
To: Phillip Susi <phill@thesusis.net>, linux-ide@vger.kernel.org
Cc: Sergey Shtylyov <s.shtylyov@omp.ru>
Subject: Re: [PATCH 1/3] libata: avoid waking disk for several commands
Date: Thu, 25 Jan 2024 06:51:39 +0900 [thread overview]
Message-ID: <7e324bce-9984-4291-8b5f-0907483e7bc1@kernel.org> (raw)
In-Reply-To: <875xziiuou.fsf@vps.thesusis.net>
On 1/25/24 01:04, Phillip Susi wrote:
> Damien Le Moal <dlemoal@kernel.org> writes:
>
>> Flush issuing is a lot more complicated than just blkdev_issue_flush(). There is
>> a whole file dedicated to handling flushes (block/blk-flush.c).
>>
>> But that is beside the point, which is that trying to not execute flush is
>> simply completely wrong. Please stop trying.
>
> I tried before to have libata ignore the useless flush and you said to
> stop the flush from happening in the first place. Now you say that's wrong?
No, not wrong. But you are looking at the wrong layer. Do not try to prevent
flushes from being issued at the block layer level but *above* it. That is,
FSes, applications etc.
>> For your case, which is a drive put to sleep with hdparm -Y, only libata is
>> aware that the drive is sleeping. That first needs to change if you want the
>> kernel overall to be able to do anything. As I proposed already, using runtime
>> PM with sleep mode instead of standby would be a good start.
>
> No, I'm working on runtime pm now, as you suggested. If you try using
> runtime pm with disks, you quickly see that it does not work.
What does not work ? Everything is fine with my testing: the drive is always
woken up whenever someone (FS, applications etc) issue a block IO (including
flush) to the block layer. That is the expected behavior. If you want to have
the disk keep sleeping, the device users must stop poking at the drive.
>
>> Regarding the flushes and other commands you see that are waking up the drive
>> for no *apparent* good reasons, please identify what application/component is
>> issuing these commands and look there to see why the commands are being issued
>> and if that is done with awareness for the device power state.
>
> Filesystems flush every few seconds. So does anyone calling sync(),
> which the kernel does when you suspend to ram.
Filesystems issue flush only if they have dirty data to flush, normally. I have
not looked at ext4/xfs code in a while, but if the FS has not committed any
transaction in the last cycle, there should be no flush issued. If there is,
then someone is reading or writing files (for reading, if you have atime
enabled, that will cause a metadata change and so a transaction commit).
> Either the filesystems need to keep track of whether a flush is needed
> and skip it, or if they all call the same place ( blkdev_issue_flush ),
> then it only needs done once in that place.
See above. That is why I am telling you to run blktrace or any tracer to figure
out the command sequence and who is doing what. There is absolutely no clarity
to what you are describing and so we cannot plan for a solution.
> The core logic needs to be "if nothing has been written, then nothing
> needs to be flushed". Right now filesystems just flush periodically or
> when their sync method is called to make sure that anything that has
> been written is stable.
I do not think that the periodic flush happens if nothing has been written. For
"sync", someone issues that, not the FS on its own. So trace it to find out who
is doing that.
> In userspace you have smartd and udisks2 to worry about. The
> information they need to know is whether the disk has otherwise been in
> recent use and so they should not poll the SMART data. I don't see
> anything exported in the power/ directory that would give an indication
> of the current *remaining* time until autosuspend, or how long it has
> been since the last access. Either one would be useful for userspace to
> decide that nobody else seems to be using the disk, so I'll leave it
> alone for now so it can go to sleep.
--
Damien Le Moal
Western Digital Research
next prev parent reply other threads:[~2024-01-24 21:51 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-25 15:19 [PATCH 0/1] Only activate drive once during system resume Phillip Susi
2023-12-25 15:19 ` [PATCH 1/1] libata: only wake a drive once on " Phillip Susi
2023-12-30 18:21 ` [PATCH 0/1 v2] Only activate drive once during " Phillip Susi
2023-12-30 18:21 ` [PATCH 1/1] libata: only wake a drive once on " Phillip Susi
2023-12-30 19:42 ` Sergey Shtylyov
2024-01-02 23:17 ` Damien Le Moal
2024-01-03 21:00 ` Phillip Susi
2024-01-04 1:21 ` Damien Le Moal
2024-01-04 14:05 ` Phillip Susi
2024-01-04 22:39 ` [PATCH 1/4] " Phillip Susi
2024-01-04 22:39 ` [PATCH 2/4] libata: don't wake sleeping disk during system suspend Phillip Susi
2024-01-05 12:25 ` Damien Le Moal
2024-01-05 16:18 ` Phillip Susi
2024-01-04 22:39 ` [PATCH 3/4] libata: avoid waking disk for several commands Phillip Susi
2024-01-05 8:46 ` Sergei Shtylyov
2024-01-05 16:24 ` Phillip Susi
2024-01-05 18:33 ` Sergei Shtylyov
2024-01-06 19:49 ` Phillip Susi
2024-01-06 20:29 ` Phillip Susi
2024-01-08 8:57 ` Sergei Shtylyov
2024-01-05 12:29 ` Damien Le Moal
2024-01-05 16:30 ` Phillip Susi
2024-01-06 23:14 ` Damien Le Moal
2024-01-07 17:57 ` Phillip Susi
2024-01-07 18:02 ` [PATCH 0/3] Let sleeping disks lie Phillip Susi
2024-01-07 18:02 ` [PATCH 1/3] libata: avoid waking disk for several commands Phillip Susi
2024-01-08 6:25 ` Damien Le Moal
2024-01-08 13:27 ` Phillip Susi
2024-01-10 2:39 ` Damien Le Moal
2024-01-16 17:06 ` Phillip Susi
2024-01-19 20:43 ` Phillip Susi
2024-01-20 18:08 ` Phillip Susi
2024-01-21 0:37 ` Damien Le Moal
2024-01-21 0:37 ` Damien Le Moal
2024-01-24 16:04 ` Phillip Susi
2024-01-24 21:51 ` Damien Le Moal [this message]
2024-02-01 20:01 ` Phillip Susi
2024-02-02 1:08 ` Damien Le Moal
2024-02-02 19:53 ` Phillip Susi
2024-02-02 23:17 ` Damien Le Moal
2024-02-05 19:52 ` Phillip Susi
2024-01-08 8:48 ` Sergey Shtylyov
2024-01-08 13:30 ` Phillip Susi
2024-01-07 18:02 ` [PATCH 2/3] libata: only wake a drive once on system resume Phillip Susi
2024-01-08 6:04 ` Damien Le Moal
2024-01-07 18:02 ` [PATCH 3/3] libata: don't start PuiS disks on resume Phillip Susi
2024-01-08 6:03 ` Damien Le Moal
2024-01-08 13:39 ` Phillip Susi
2024-01-10 2:19 ` Damien Le Moal
2024-01-16 17:13 ` Phillip Susi
2024-01-04 22:39 ` [PATCH 4/4] " Phillip Susi
2024-01-05 8:57 ` Sergei Shtylyov
2024-01-05 12:42 ` Damien Le Moal
2024-01-05 16:44 ` Phillip Susi
2024-01-05 12:13 ` [PATCH 1/4] libata: only wake a drive once on system resume Damien Le Moal
2024-01-05 17:03 ` Phillip Susi
2024-01-06 23:06 ` Damien Le Moal
2024-01-05 12:44 ` Damien Le Moal
2024-01-09 15:20 ` [PATCH 0/1 v2] Only activate drive once during " Niklas Cassel
2024-01-16 17:23 ` Phillip Susi
2024-01-02 22:46 ` [PATCH 0/1] " Damien Le Moal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7e324bce-9984-4291-8b5f-0907483e7bc1@kernel.org \
--to=dlemoal@kernel.org \
--cc=linux-ide@vger.kernel.org \
--cc=phill@thesusis.net \
--cc=s.shtylyov@omp.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).