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    [W:0.021 / U:0.100 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site