lkml.org 
[lkml]   [2009]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 05/12] writeback: switch to per-bdi threads for flushing data
    On Tue 31-03-09 18:50:40, Jens Axboe wrote:
    > On Tue, Mar 31 2009, Jan Kara wrote:
    > > > This gets rid of pdflush for bdi writeout and kupdated style cleaning.
    > > > This is an experiment to see if we get better writeout behaviour with
    > > > per-bdi flushing. Some initial tests look pretty encouraging. A sample
    > > > ffsb workload that does random writes to files is about 8% faster here
    > > > on a simple SATA drive during the benchmark phase. File layout also seems
    > > > a LOT more smooth in vmstat:
    > > >
    > > > r b swpd free buff cache si so bi bo in cs us sy id wa
    > > > 0 1 0 608848 2652 375372 0 0 0 71024 604 24 1 10 48 42
    > > > 0 1 0 549644 2712 433736 0 0 0 60692 505 27 1 8 48 44
    > > > 1 0 0 476928 2784 505192 0 0 4 29540 553 24 0 9 53 37
    > > > 0 1 0 457972 2808 524008 0 0 0 54876 331 16 0 4 38 58
    > > > 0 1 0 366128 2928 614284 0 0 4 92168 710 58 0 13 53 34
    > > > 0 1 0 295092 3000 684140 0 0 0 62924 572 23 0 9 53 37
    > > > 0 1 0 236592 3064 741704 0 0 4 58256 523 17 0 8 48 44
    > > > 0 1 0 165608 3132 811464 0 0 0 57460 560 21 0 8 54 38
    > > > 0 1 0 102952 3200 873164 0 0 4 74748 540 29 1 10 48 41
    > > > 0 1 0 48604 3252 926472 0 0 0 53248 469 29 0 7 47 45
    > > >
    > > > where vanilla tends to fluctuate a lot in the creation phase:
    > > >
    > > > r b swpd free buff cache si so bi bo in cs us sy id wa
    > > > 1 1 0 678716 5792 303380 0 0 0 74064 565 50 1 11 52 36
    > > > 1 0 0 662488 5864 319396 0 0 4 352 302 329 0 2 47 51
    > > > 0 1 0 599312 5924 381468 0 0 0 78164 516 55 0 9 51 40
    > > > 0 1 0 519952 6008 459516 0 0 4 78156 622 56 1 11 52 37
    > > > 1 1 0 436640 6092 541632 0 0 0 82244 622 54 0 11 48 41
    > > > 0 1 0 436640 6092 541660 0 0 0 8 152 39 0 0 51 49
    > > > 0 1 0 332224 6200 644252 0 0 4 102800 728 46 1 13 49 36
    > > > 1 0 0 274492 6260 701056 0 0 4 12328 459 49 0 7 50 43
    > > > 0 1 0 211220 6324 763356 0 0 0 106940 515 37 1 10 51 39
    > > > 1 0 0 160412 6376 813468 0 0 0 8224 415 43 0 6 49 45
    > > > 1 1 0 85980 6452 886556 0 0 4 113516 575 39 1 11 54 34
    > > > 0 2 0 85968 6452 886620 0 0 0 1640 158 211 0 0 46 54
    > > >
    > > > So apart from seemingly behaving better for buffered writeout, this also
    > > > allows us to potentially have more than one bdi thread flushing out data.
    > > > This may be useful for NUMA type setups.
    > > >
    > > > A 10 disk test with btrfs performs 26% faster with per-bdi flushing. Other
    > > > tests pending.
    > > >
    > > > Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
    > > Two comments below...
    > >
    > > <snip>
    > > > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
    > > > index 45d2739..21de5ac 100644
    > > > --- a/fs/fs-writeback.c
    > > > +++ b/fs/fs-writeback.c
    > > > @@ -19,6 +19,8 @@
    > > > #include <linux/sched.h>
    > > > #include <linux/fs.h>
    > > > #include <linux/mm.h>
    > > > +#include <linux/kthread.h>
    > > > +#include <linux/freezer.h>
    > > > #include <linux/writeback.h>
    > > > #include <linux/blkdev.h>
    > > > #include <linux/backing-dev.h>
    > > > @@ -61,10 +63,180 @@ int writeback_in_progress(struct backing_dev_info *bdi)
    > > > */
    > > > static void writeback_release(struct backing_dev_info *bdi)
    > > > {
    > > > - BUG_ON(!writeback_in_progress(bdi));
    > > > + WARN_ON_ONCE(!writeback_in_progress(bdi));
    > > > + bdi->wb_arg.nr_pages = 0;
    > > > + bdi->wb_arg.sb = NULL;
    > > > clear_bit(BDI_pdflush, &bdi->state);
    > > > }
    > > >
    > > > +int bdi_start_writeback(struct backing_dev_info *bdi, struct super_block *sb,
    > > > + long nr_pages)
    > > > +{
    > > > + /*
    > > > + * This only happens the first time someone kicks this bdi, so put
    > > > + * it out-of-line.
    > > > + */
    > > > + if (unlikely(!bdi->task)) {
    > > > + bdi_add_flusher_task(bdi);
    > > > + return 1;
    > > > + }
    > > > +
    > > > + if (writeback_acquire(bdi)) {
    > > > + bdi->wb_arg.nr_pages = nr_pages;
    > > > + bdi->wb_arg.sb = sb;
    > > > + /*
    > > > + * make above store seen before the task is woken
    > > > + */
    > > > + smp_mb();
    > > > + wake_up(&bdi->wait);
    > > > + }
    > > > +
    > > > + return 0;
    > > > +}
    > > > +
    > > > +/*
    > > > + * The maximum number of pages to writeout in a single bdi flush/kupdate
    > > > + * operation. We do this so we don't hold I_SYNC against an inode for
    > > > + * enormous amounts of time, which would block a userspace task which has
    > > > + * been forced to throttle against that inode. Also, the code reevaluates
    > > > + * the dirty each time it has written this many pages.
    > > > + */
    > > > +#define MAX_WRITEBACK_PAGES 1024
    > > > +
    > > > +/*
    > > > + * Periodic writeback of "old" data.
    > > > + *
    > > > + * Define "old": the first time one of an inode's pages is dirtied, we mark the
    > > > + * dirtying-time in the inode's address_space. So this periodic writeback code
    > > > + * just walks the superblock inode list, writing back any inodes which are
    > > > + * older than a specific point in time.
    > > > + *
    > > > + * Try to run once per dirty_writeback_interval. But if a writeback event
    > > > + * takes longer than a dirty_writeback_interval interval, then leave a
    > > > + * one-second gap.
    > > > + *
    > > > + * older_than_this takes precedence over nr_to_write. So we'll only write back
    > > > + * all dirty pages if they are all attached to "old" mappings.
    > > > + */
    > > > +static void bdi_kupdated(struct backing_dev_info *bdi)
    > > > +{
    > > > + long nr_to_write;
    > > > + struct writeback_control wbc = {
    > > > + .bdi = bdi,
    > > > + .sync_mode = WB_SYNC_NONE,
    > > > + .nr_to_write = 0,
    > > > + .for_kupdate = 1,
    > > > + .range_cyclic = 1,
    > > > + };
    > > > +
    > > > + sync_supers();
    > > > +
    > > > + nr_to_write = global_page_state(NR_FILE_DIRTY) +
    > > > + global_page_state(NR_UNSTABLE_NFS) +
    > > > + (inodes_stat.nr_inodes - inodes_stat.nr_unused);
    > > > +
    > > > + while (nr_to_write > 0) {
    > > > + wbc.more_io = 0;
    > > > + wbc.encountered_congestion = 0;
    > > > + wbc.nr_to_write = MAX_WRITEBACK_PAGES;
    > > > + generic_sync_bdi_inodes(NULL, &wbc);
    > > > + if (wbc.nr_to_write > 0)
    > > > + break; /* All the old data is written */
    > > > + nr_to_write -= MAX_WRITEBACK_PAGES;
    > > > + }
    > > > +}
    > > > +
    > > > +static void bdi_pdflush(struct backing_dev_info *bdi)
    > > > +{
    > > > + struct writeback_control wbc = {
    > > > + .bdi = bdi,
    > > > + .sync_mode = WB_SYNC_NONE,
    > > > + .older_than_this = NULL,
    > > > + .range_cyclic = 1,
    > > > + };
    > > > + long nr_pages = bdi->wb_arg.nr_pages;
    > > > +
    > > > + for (;;) {
    > > > + unsigned long background_thresh, dirty_thresh;
    > > > + get_dirty_limits(&background_thresh, &dirty_thresh, NULL, NULL);
    > > > + if ((global_page_state(NR_FILE_DIRTY) +
    > > > + global_page_state(NR_UNSTABLE_NFS) < background_thresh) &&
    > > > + nr_pages <= 0)
    > > > + break;
    > > > +
    > > > + wbc.more_io = 0;
    > > > + wbc.encountered_congestion = 0;
    > > > + wbc.nr_to_write = MAX_WRITEBACK_PAGES;
    > > > + wbc.pages_skipped = 0;
    > > > + generic_sync_bdi_inodes(bdi->wb_arg.sb, &wbc);
    > > > + nr_pages -= MAX_WRITEBACK_PAGES - wbc.nr_to_write;
    > > > + /*
    > > > + * If we ran out of stuff to write, bail unless more_io got set
    > > > + */
    > > > + if (wbc.nr_to_write > 0 || wbc.pages_skipped > 0) {
    > > > + if (wbc.more_io)
    > > > + continue;
    > > > + break;
    > > > + }
    > > > + }
    > > > +}
    > > > +
    > > > +/*
    > > > + * Handle writeback of dirty data for the device backed by this bdi. Also
    > > > + * wakes up periodically and does kupdated style flushing.
    > > > + */
    > > > +int bdi_writeback_task(struct backing_dev_info *bdi)
    > > > +{
    > > > + while (!kthread_should_stop()) {
    > > > + DEFINE_WAIT(wait);
    > > > +
    > > > + prepare_to_wait(&bdi->wait, &wait, TASK_INTERRUPTIBLE);
    > > > + schedule_timeout(dirty_writeback_interval);
    > > > + try_to_freeze();
    > > > +
    > > > + /*
    > > > + * We get here in two cases:
    > > > + *
    > > > + * schedule_timeout() returned because the dirty writeback
    > > > + * interval has elapsed. If that happens, we will be able
    > > > + * to acquire the writeback lock and will proceed to do
    > > > + * kupdated style writeout.
    > > > + *
    > > > + * Someone called bdi_start_writeback(), which will acquire
    > > > + * the writeback lock. This means our writeback_acquire()
    > > > + * below will fail and we call into bdi_pdflush() for
    > > > + * pdflush style writeout.
    > > > + *
    > > > + */
    > > > + if (writeback_acquire(bdi))
    > > > + bdi_kupdated(bdi);
    > > > + else
    > > > + bdi_pdflush(bdi);
    > > > +
    > > > + writeback_release(bdi);
    > > > + finish_wait(&bdi->wait, &wait);
    > > > + }
    > > > +
    > > > + return 0;
    > > > +}
    > > > +
    > > > +void bdi_writeback_all(struct super_block *sb, long nr_pages)
    > > > +{
    > > > + struct backing_dev_info *bdi;
    > > > +
    > > > + rcu_read_lock();
    > > > +
    > > > +restart:
    > > > + list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) {
    > > > + if (!bdi_has_dirty_io(bdi))
    > > > + continue;
    > > > + if (bdi_start_writeback(bdi, sb, nr_pages))
    > > > + goto restart;
    > > > + }
    > > > +
    > > > + rcu_read_unlock();
    > > > +}
    > > > +
    > > > /**
    > > > * __mark_inode_dirty - internal function
    > > > * @inode: inode to mark
    > > > @@ -248,46 +420,6 @@ static void queue_io(struct backing_dev_info *bdi,
    > > > move_expired_inodes(&bdi->b_dirty, &bdi->b_io, older_than_this);
    > > > }
    > > >
    > > > -static int sb_on_inode_list(struct super_block *sb, struct list_head *list)
    > > > -{
    > > > - struct inode *inode;
    > > > - int ret = 0;
    > > > -
    > > > - spin_lock(&inode_lock);
    > > > - list_for_each_entry(inode, list, i_list) {
    > > > - if (inode->i_sb == sb) {
    > > > - ret = 1;
    > > > - break;
    > > > - }
    > > > - }
    > > > - spin_unlock(&inode_lock);
    > > > - return ret;
    > > > -}
    > > > -
    > > > -int sb_has_dirty_inodes(struct super_block *sb)
    > > > -{
    > > > - struct backing_dev_info *bdi;
    > > > - int ret = 0;
    > > > -
    > > > - /*
    > > > - * This is REALLY expensive right now, but it'll go away
    > > > - * when the bdi writeback is introduced
    > > > - */
    > > > - rcu_read_lock();
    > > > - list_for_each_entry_rcu(bdi, &bdi_list, bdi_list) {
    > > > - if (sb_on_inode_list(sb, &bdi->b_dirty) ||
    > > > - sb_on_inode_list(sb, &bdi->b_io) ||
    > > > - sb_on_inode_list(sb, &bdi->b_more_io)) {
    > > > - ret = 1;
    > > > - break;
    > > > - }
    > > > - }
    > > > - rcu_read_unlock();
    > > > -
    > > > - return ret;
    > > > -}
    > > > -EXPORT_SYMBOL(sb_has_dirty_inodes);
    > > > -
    > > > /*
    > > > * Write a single inode's dirty pages and inode data out to disk.
    > > > * If `wait' is set, wait on the writeout.
    > > > @@ -446,10 +578,11 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
    > > > return __sync_single_inode(inode, wbc);
    > > > }
    > > >
    > > > -static void generic_sync_bdi_inodes(struct backing_dev_info *bdi,
    > > > - struct writeback_control *wbc,
    > > > - int is_blkdev)
    > > > +void generic_sync_bdi_inodes(struct super_block *sb,
    > > > + struct writeback_control *wbc)
    > > > {
    > > > + const int is_blkdev_sb = sb_is_blkdev_sb(sb);
    > > > + struct backing_dev_info *bdi = wbc->bdi;
    > > > const unsigned long start = jiffies; /* livelock avoidance */
    > > >
    > > > spin_lock(&inode_lock);
    > > > @@ -461,9 +594,17 @@ static void generic_sync_bdi_inodes(struct backing_dev_info *bdi,
    > > > struct inode, i_list);
    > > > long pages_skipped;
    > > >
    > > > + /*
    > > > + * super block given and doesn't match, skip this inode
    > > > + */
    > > > + if (sb && sb != inode->i_sb) {
    > > > + redirty_tail(inode);
    > > > + continue;
    > > > + }
    > > > +
    > > > if (!bdi_cap_writeback_dirty(bdi)) {
    > > > redirty_tail(inode);
    > > > - if (is_blkdev) {
    > > > + if (is_blkdev_sb) {
    > > > /*
    > > > * Dirty memory-backed blockdev: the ramdisk
    > > > * driver does this. Skip just this inode
    > > > @@ -485,33 +626,20 @@ static void generic_sync_bdi_inodes(struct backing_dev_info *bdi,
    > > >
    > > > if (wbc->nonblocking && bdi_write_congested(bdi)) {
    > > > wbc->encountered_congestion = 1;
    > > > - if (!is_blkdev)
    > > > + if (!is_blkdev_sb)
    > > > break; /* Skip a congested fs */
    > > > requeue_io(inode);
    > > > continue; /* Skip a congested blockdev */
    > > > }
    > > >
    > > > - if (wbc->bdi && bdi != wbc->bdi) {
    > > > - if (!is_blkdev)
    > > > - break; /* fs has the wrong queue */
    > > > - requeue_io(inode);
    > > > - continue; /* blockdev has wrong queue */
    > > > - }
    > > > -
    > > > /* Was this inode dirtied after sync_sb_inodes was called? */
    > > > if (time_after(inode->dirtied_when, start))
    > > > break;
    > > >
    > > > - /* Is another pdflush already flushing this queue? */
    > > > - if (current_is_pdflush() && !writeback_acquire(bdi))
    > > > - break;
    > > > -
    > > > BUG_ON(inode->i_state & I_FREEING);
    > > > __iget(inode);
    > > > pages_skipped = wbc->pages_skipped;
    > > > __writeback_single_inode(inode, wbc);
    > > > - if (current_is_pdflush())
    > > > - writeback_release(bdi);
    > > > if (wbc->pages_skipped != pages_skipped) {
    > > > /*
    > > > * writeback is not making progress due to locked
    > > > @@ -550,11 +678,6 @@ static void generic_sync_bdi_inodes(struct backing_dev_info *bdi,
    > > > * a variety of queues, so all inodes are searched. For other superblocks,
    > > > * assume that all inodes are backed by the same queue.
    > > > *
    > > > - * FIXME: this linear search could get expensive with many fileystems. But
    > > > - * how to fix? We need to go from an address_space to all inodes which share
    > > > - * a queue with that address_space. (Easy: have a global "dirty superblocks"
    > > > - * list).
    > > > - *
    > > > * The inodes to be written are parked on bdi->b_io. They are moved back onto
    > > > * bdi->b_dirty as they are selected for writing. This way, none can be missed
    > > > * on the writer throttling path, and we get decent balancing between many
    > > > @@ -563,13 +686,10 @@ static void generic_sync_bdi_inodes(struct backing_dev_info *bdi,
    > > > void generic_sync_sb_inodes(struct super_block *sb,
    > > > struct writeback_control *wbc)
    > > > {
    > > > - const int is_blkdev = sb_is_blkdev_sb(sb);
    > > > - struct backing_dev_info *bdi;
    > > > -
    > > > - rcu_read_lock();
    > > > - list_for_each_entry_rcu(bdi, &bdi_list, bdi_list)
    > > > - generic_sync_bdi_inodes(bdi, wbc, is_blkdev);
    > > > - rcu_read_unlock();
    > > > + if (wbc->bdi)
    > > > + bdi_start_writeback(wbc->bdi, sb, 0);
    > > > + else
    > > > + bdi_writeback_all(sb, 0);
    > > Hmm, generic_sync_sb_inodes() is also used for calls like sys_sync()
    > > (i.e. data integrity sync). But bdi_start_writeback() (called also from
    > > bdi_writeback_all()) just skips the bdi if somebody is already writing
    > > it. But to ensure data integrity we have to wait for previous writer to
    > > finish (or interupt him somehow) and write it ourselves. Nasty for
    > > performance but needed for integrity...
    >
    > Good point, thanks for your reviews Jan! It may indeed be a problem, if
    Welcome.

    > the new caller is less retrictive than the one currently running. I
    > guess it's pretty easily fixable by doing a blocking wait on the
    > writeback bit for those cases, usable from
    > bdi_start_writeback_blocking() or something to that effect. I'll add
    > that for v3, it is going to be posted fairly soon.
    Yes, that should be fine. We do it similarly for I_SYNC bit on inodes -
    actually it's there for exactly above reason.

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


    \
     
     \ /
      Last update: 2009-04-01 11:15    [W:0.051 / U:31.164 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site