public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Daniel Phillips <d.phillips@partner.samsung.com>
Cc: Jan Kara <jack@suse.cz>, Christoph Hellwig <hch@infradead.org>,
	Dave Chinner <david@fromorbit.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>,
	Jens Axboe <axboe@kernel.dk>
Subject: Re: [RFC][PATCH 1/2] Add a super operation for writeback
Date: Wed, 4 Jun 2014 22:16:33 +0200	[thread overview]
Message-ID: <20140604201633.GB22927@quack.suse.cz> (raw)
In-Reply-To: <19f0e788-f6f2-4547-9cb2-a74d70d75cd0@partner.samsung.com>

On Tue 03-06-14 15:37:39, Daniel Phillips wrote:
> On Tuesday, June 3, 2014 8:21:55 AM PDT, Jan Kara wrote:
> >On Tue 03-06-14 07:14:44, Christoph Hellwig wrote:
> >>On Tue, Jun 03, 2014 at 04:05:31PM +0200, Jan Kara wrote:
> >And I agree we went for per-bdi
> >flushing to avoid two threads congesting a single device leading to
> >suboptimal IO patterns during background writeback.
> 
> A proposal is on the table to implement s_ops->writeback() as a per-sb
> operation in such a way that nothing changes in the current per-inode path.
> Good or bad approach?
  Having s_ops->writeback() is fine. But I just hate how you hook in that
callback into the writeback code (and Dave has expressed similar concerns).
The way you hook it up, filesystem still has to have one inode in the dirty
list so that __writeback_inodes_wb() can find that inode, get superblock
from it and ask for writeback to be done via callback. That's really ugly
and no-go for me.

> >So currently I'm convinced we want to go for per-sb dirty tracking. That
> >also makes some speedups in that code noticeably simpler. I'm not
> convinced
> >about the per-sb flushing thread - if we don't regress the multiple sb on
> >bdi case when we just let the threads from different superblocks contend
> >for IO, then that would be a natural thing to do. But once we have to
> >introduce some synchronization between threads to avoid regressions, I
> >think it might be easier to just stay with per-bdi thread which switches
> >between superblocks.
> 
> Could you elaborate on the means of switching between superblocks?  Do
> you mean a new fs-writeback path just for data=journal class filesystems,
> or are you suggesting changing the way all filesystems are driven?
  So I suggest changing the way all filesystems are driven to a per-sb one.
That makes sense for other reasons as well and allows clean incorporation
of your writeback callback (either a fs will use generic callback which
would do what generic writeback code does now, only with per-sb dirty lists
instead of current per-bdi ones, or the fs can do its own stuff). I can
write something (the switch isn't really that complex) but it will need
quite some testing to verify we don't regress somewhere...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  reply	other threads:[~2014-06-04 20:16 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-01 21:41 [RFC][PATCH 1/2] Add a super operation for writeback Daniel Phillips
2014-06-01 21:42 ` [RFC][PATCH 2/2] tux3: Use writeback hook to remove duplicated core code Daniel Phillips
2014-06-02  3:30   ` Dave Chinner
2014-06-02 20:07     ` Daniel Phillips
2014-06-02  3:15 ` [RFC][PATCH 1/2] Add a super operation for writeback Dave Chinner
2014-06-02 20:02   ` Daniel Phillips
2014-06-03  3:33     ` Dave Chinner
2014-06-03  7:01       ` Daniel Phillips
2014-06-03  7:26         ` Daniel Phillips
2014-06-03  7:47         ` OGAWA Hirofumi
2014-06-03  8:12           ` Dave Chinner
2014-06-03  8:57             ` OGAWA Hirofumi
2014-06-03  7:52         ` Dave Chinner
2014-06-03 14:05           ` Jan Kara
2014-06-03 14:14             ` Christoph Hellwig
2014-06-03 14:25               ` Theodore Ts'o
2014-06-03 15:21               ` Jan Kara
2014-06-03 22:37                 ` Daniel Phillips
2014-06-04 20:16                   ` Jan Kara [this message]
2014-06-02  8:30 ` Christian Stroetmann
2014-06-03  3:39   ` Dave Chinner
2014-06-03  5:30     ` Christian Stroetmann
2014-06-03 14:57       ` Theodore Ts'o
2014-06-03 16:30         ` Christian Stroetmann

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=20140604201633.GB22927@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=d.phillips@partner.samsung.com \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=hirofumi@mail.parknet.co.jp \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.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