All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	Linux FS Maling List <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Maling List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/4] hfsplus: get rid of write_super
Date: Thu, 21 Jun 2012 23:30:56 +0300	[thread overview]
Message-ID: <1340310656.2536.12.camel@koala> (raw)
In-Reply-To: <20120621124158.a7559ee3.akpm@linux-foundation.org>

[-- Attachment #1: Type: text/plain, Size: 2301 bytes --]

On Thu, 2012-06-21 at 12:41 -0700, Andrew Morton wrote:
> > +	spin_lock(&sbi->work_lock);
> > +	if (!sbi->work_queued) {
> > +	       delay = msecs_to_jiffies(dirty_writeback_interval * 10);
> > +	       queue_delayed_work(system_long_wq, &sbi->sync_work, delay);
> > +	       sbi->work_queued = 1;
> > +	}
> > +	spin_unlock(&sbi->work_lock);
> >  }
> 
> And I think it could be made to go away here, perhaps by switching to
> test_and_set_bit or similar.

Yes, you are right, and I thought about this. But I did not want to make
it more complicated than needed. Spinlock is simple and
straight-forward, this is not a fast-path.

> And I wonder about the queue_delayed_work().  iirc this does nothing to
> align timer expiries, so someone who has a lot of filesystems could end
> up with *more* timer wakeups.  Shouldn't we do something here to make
> the system do larger amounts of work per timer expiry?  Such as the
> timer-slack infrastructure?

Well, I also thought about this. But I again did not want to invent
anything complex because main file systems - ext4, btrfs, xfs - do not
use 'write_super()' at all. And then only these dying / rare
file-systems like btrfs / hfs - I did not feel like over-engineering is
needed.

If someone is affected by more timers, which I really really doubt, we
can further optimize this using deferrable timers for some file-systems
which do not really think anything super-important, and we can use range
hrtimers for file-systems which sync something more or less important.
The former is easy to do, the latter would need improving the workqueue
infrastructure.


> It strikes me that this whole approach improves the small system with
> little write activity, but makes things worse for the larger system
> with a lot of filesystems?

Well, if we have a lot of those rare FSes and do not have much
activities, then we do not wake up. If we have a lot of activities,
probably it does not hurt to wake-up much. But of course there is a
situation when this would hurt, but I again, do doubt such systems are
common, an really care.

IOW, I do not dismiss your point, but I think that we rather see if it
affects anyone and then optimize with a deferrable/range timers.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2012-06-21 20:30 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-13 11:37 [PATCH 0/4] hfsplus: stop using write_supers and s_dirt Artem Bityutskiy
2012-06-13 11:37 ` [PATCH 1/4] hfsplus: make hfsplus_sync_fs static Artem Bityutskiy
2012-06-13 11:37 ` [PATCH 2/4] hfsplus: amend debugging print Artem Bityutskiy
2012-06-13 11:37 ` [PATCH 3/4] hfsplus: remove useless check Artem Bityutskiy
2012-06-13 11:37 ` [PATCH 4/4] hfsplus: get rid of write_super Artem Bityutskiy
2012-06-21 19:41   ` Andrew Morton
2012-06-21 20:30     ` Artem Bityutskiy [this message]
2012-06-21 20:53       ` Artem Bityutskiy
2012-07-02 14:13   ` Artem Bityutskiy
2012-06-21 12:16 ` [PATCH 0/4] hfsplus: stop using write_supers and s_dirt 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=1340310656.2536.12.camel@koala \
    --to=dedekind1@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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.