lkml.org 
[lkml]   [2010]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv4 17/17] writeback: lessen sync_supers wakeup count
On Thu, May 27, 2010 at 07:50:41AM +0100, Al Viro wrote:
> On Tue, May 25, 2010 at 04:49:12PM +0300, Artem Bityutskiy wrote:
> > From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> >
> > The 'sync_supers' thread wakes up every 5 seconds (by default) and
> > writes back all super blocks. It keeps waking up even if there
> > are no dirty super-blocks. For many file-systems the superblock
> > becomes dirty very rarely, if ever, so 'sync_supers' does not do
> > anything most of the time.
> >
> > This patch improves 'sync_supers' and makes sleep if all superblocks
> > are clean and there is nothing to do. This helps saving the power.
> > This optimization is important for small battery-powered devices.
>
> > +void mark_sb_dirty(struct super_block *sb)
> > +{
> > + sb->s_dirty = 1;
> > +
> > + spin_lock(&supers_timer_lock);
> > + if (!supers_timer_armed) {
> > + bdi_arm_supers_timer();
> > + supers_timer_armed = 1;
> > + } else if (supers_timer_armed == -1)
> > + supers_timer_armed = 1;
> > + spin_unlock(&supers_timer_lock);
> > +}
> > +EXPORT_SYMBOL(mark_sb_dirty);
>
> Ouch... That turns a previously trivial operation into something
> much heavier; moreover, I'd rather see serious review of s_dirt
> uses.

Yeah, we definitely don't want to add global cacheline writes in the
common case. Also I don't know why you do the strange -1 value. I
couldn't seem to find where you defined bdi_arm_supers_timer();

But why doesn't this work?

sb->s_dirty = 1;
smp_mb(); /* corresponding MB is in test_and_clear_bit */
if (unlikely(!supers_timer_armed)) {
if (!test_and_set_bit(0, &supers_timer_armed))
bdi_arm_supers_timer();
}
vs

supers_timer_armed = 0;
again:
sync_supers();
if (test_and_clear_bit(0, &supers_timer_armed))
goto again;


\
 
 \ /
  Last update: 2010-05-27 09:25    [from the cache]
©2003-2011 Jasper Spaans