From: Nick Piggin <npiggin@suse.de>
To: Tero.Kristo@nokia.com
Cc: dedekind1@gmail.com, npiggin@suse.de, axboe@kernel.dk,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv2 11/11] writeback: prevent unnecessary bdi threads wakeups
Date: Thu, 22 Jul 2010 18:07:06 +1000 [thread overview]
Message-ID: <20100722080706.GB9377@amd> (raw)
In-Reply-To: <1F18D6510CF0474A8C9500565A7E41A22D4D24C4C2@NOK-EUMSG-02.mgdnok.nokia.com>
On Thu, Jul 22, 2010 at 09:22:16AM +0200, Tero.Kristo@nokia.com wrote:
> >-----Original Message-----
> >From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
> >Sent: 22 July, 2010 09:48
> >To: Nick Piggin; Kristo Tero (Nokia-MS/Tampere)
> >Cc: Jens Axboe; linux-fsdevel@vger.kernel.org; linux-
> >kernel@vger.kernel.org
> >Subject: Re: [PATCHv2 11/11] writeback: prevent unnecessary bdi threads
> >wakeups
> >
> >Hi Nick,
> >
> >On Thu, 2010-07-22 at 13:19 +1000, Nick Piggin wrote:
> >> > out:
> >> > spin_unlock(&inode_lock);
> >> > +
> >> > + if (wakeup_bdi) {
> >> > + spin_lock(&bdi->wb_lock);
> >> > + if (!bdi->wb.task)
> >> > + wake_up_process(default_backing_dev_info.wb.task);
> >> > + else
> >> > + wake_up_process(bdi->wb.task);
> >> > + spin_unlock(&bdi->wb_lock);
> >> > + }
> >> > }
> >>
> >> We really want to wake up the bdi right away when first dirtying
> >> the inode? I haven't looked at where the state of the bdi code is
> >> now, but isn't it better to have a a delay there?
> >
> >Yes, I guess we want to wake up the bdi thread after 5 secs (assuming
> >default settings). I could implement a "wake_up_process_delayed"
> >function which would use a timer, but I think it is not necessary to
> >introduce these complications. We can just wake-up the bdi thread, it'll
> >find out there is nothing to do, and will go sleep for 5 secs. I think
> >this is good enough.
> >
> >IOW, delayed wake-up is not worth the trouble.
> >
> >> And rather than spreading details of how bdi tasks are managed
> >> would you consider putting this into its own function?
> >
> >Sure, will do.
> >
> >> Other than that, I like your patches.
> >
> >Thanks :-)
> >
> >> Out of interest, is 5 seconds
> >> very detremental to power usage? What is a reasonable goal for
> >> wakeups? (eg. 95%+ of possible efficiency)
> >
> >I cannot tell for sure. In Nokia N900 phone we use OMAP3 and we have
> >dynamic OFF-mode, so we switch off the CPU and peripherals completely
> >when there is nothing to do, and SDRAM stays in low-power auto-refresh
> >mode. Every useless wake-up makes us do a lot of job re-constructing the
> >CPU state. I cannot tell the numbers, but I'm CCing Tero, who is working
> >on OMAP3 PM and makes a lot of battery current measurements, he can
> >provide some numbers.
>
> Well, it is hard to give any good guidelines here, as it really
> depends on the device architecture, possible power saving modes etc.,
> but I can give some sort of guestimate. Basically I think kernel
> should not generate any extra wakeups at all if it is not doing
> "anything too useful". In ideal world, everything should be interrupt
> driven as much as possible, and we would only have timers for things
> that are clearly visible for user, or can cause some sort of failure
> if neglected. Like if we ignore watchdogs, the device will reset
> itself.
>
> 5 seconds by itself is not that bad, the reason we want to get rid of
> these is that every single wakeup source cumulates. If we have 2
> wakeups occurring at 5 second intervals and they are not synced, we
> effectively can wakeup every 2.5 seconds and so on. I guess a good
> target is to have 1 device level wakeup every 30 seconds or so, but
> due to cumulation, I tend to complain about anything that happens more
> often than once a minute.
Thanks, I'm just interested in a rough idea. I agree that it would be
better to eliminate polling timeouts completely where possible.
next prev parent reply other threads:[~2010-07-22 8:07 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-21 9:31 [PATCHv2 00/16] kill unnecessary bdi wakeups + cleanups Artem Bityutskiy
2010-07-21 9:31 ` [PATCHv2 01/11] writeback: harmonize writeback threads naming Artem Bityutskiy
2010-07-21 9:31 ` [PATCHv2 02/11] writeback: fix possible race when creating bdi threads Artem Bityutskiy
2010-07-21 11:57 ` Christoph Hellwig
2010-07-21 9:31 ` [PATCHv2 03/11] writeback: do not lose wake-ups in the forker thread - 1 Artem Bityutskiy
2010-07-21 9:31 ` [PATCHv2 04/11] writeback: do not lose wake-ups in the forker thread - 2 Artem Bityutskiy
2010-07-21 11:57 ` Christoph Hellwig
2010-07-21 9:31 ` [PATCHv2 05/11] writeback: do not lose wake-ups in bdi threads Artem Bityutskiy
2010-07-21 9:31 ` [PATCHv2 06/11] writeback: simplify bdi code a little Artem Bityutskiy
2010-07-21 12:01 ` Christoph Hellwig
2010-07-21 9:31 ` [PATCHv2 07/11] writeback: do not remove bdi from bdi_list Artem Bityutskiy
2010-07-21 12:02 ` Christoph Hellwig
2010-07-21 9:31 ` [PATCHv2 08/11] writeback: move last_active to bdi Artem Bityutskiy
2010-07-21 9:31 ` [PATCHv2 09/11] writeback: restructure bdi forker loop a little Artem Bityutskiy
2010-07-21 12:02 ` Christoph Hellwig
2010-07-21 9:31 ` [PATCHv2 10/11] writeback: move bdi threads exiting logic to the forker thread Artem Bityutskiy
2010-07-21 12:04 ` Christoph Hellwig
2010-07-21 9:31 ` [PATCHv2 11/11] writeback: prevent unnecessary bdi threads wakeups Artem Bityutskiy
2010-07-21 11:45 ` Artem Bityutskiy
2010-07-22 0:41 ` Dave Chinner
2010-07-22 6:50 ` Artem Bityutskiy
2010-07-22 6:50 ` Artem Bityutskiy
2010-07-22 9:00 ` Christoph Hellwig
2010-07-22 9:24 ` Artem Bityutskiy
2010-07-22 13:27 ` Artem Bityutskiy
2010-07-22 13:27 ` Artem Bityutskiy
2010-07-21 12:12 ` Christoph Hellwig
2010-07-22 3:19 ` Nick Piggin
2010-07-22 6:48 ` Artem Bityutskiy
2010-07-22 7:22 ` Tero.Kristo
2010-07-22 7:22 ` Tero.Kristo
2010-07-22 8:07 ` Nick Piggin [this message]
2010-07-22 8:05 ` Nick Piggin
2010-07-22 8:02 ` Artem Bityutskiy
2010-07-22 8:02 ` Artem Bityutskiy
2010-07-22 8:59 ` Nick Piggin
2010-07-22 9:50 ` Artem Bityutskiy
2010-07-22 9:50 ` Artem Bityutskiy
2010-07-23 15:03 ` Artem Bityutskiy
2010-07-23 15:03 ` Artem Bityutskiy
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=20100722080706.GB9377@amd \
--to=npiggin@suse.de \
--cc=Tero.Kristo@nokia.com \
--cc=axboe@kernel.dk \
--cc=dedekind1@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.