public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
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.


  reply	other threads:[~2010-07-22  8:07 UTC|newest]

Thread overview: 34+ 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  9:00       ` Christoph Hellwig
2010-07-22  9:24         ` 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  8:07         ` Nick Piggin [this message]
2010-07-22  8:05       ` Nick Piggin
2010-07-22  8:02         ` Artem Bityutskiy
2010-07-22  8:59           ` Nick Piggin
2010-07-22  9:50         ` 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox