All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Jens Axboe <jens.axboe@oracle.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Subject: bdi forker
Date: Wed, 07 Jul 2010 14:30:27 +0300	[thread overview]
Message-ID: <1278502227.29620.161.camel@localhost> (raw)

Hi Jens,

in your current design you have:

1. bdi_forker_task() which wakes up every 5 seconds just to check
whether it has to fork a per-bdi thread or not.

2. per-bdi threads which wake up when there are bdi works or every 5
seconds to do kupdated style write-back. These threads also die after 5
minutes of "inactivity", and will be forked by the bdi_forker_task() if
needed.

This is bad from the power management POW, and I'd like to fix this. If
you have any ideas/directions for me, please, share.

I was looking at the code and and found it to bee more complex that it
has to be. I'd suggest to:

1. Stop exiting per-bdi flushers after 5 minutes of inactivity.
2. Kill the forker thread and make the default bdi us the same flusher
thread as every other bdi uses.

This will simplify many things, I believe. Various lists and locks and
will go away, the code will become more readable.

Also, I noticed the following code fragment:

static void bdi_queue_work(struct backing_dev_info *bdi,
                struct wb_writeback_work *work)
{
        spin_lock(&bdi->wb_lock);
        list_add_tail(&work->list, &bdi->work_list);
        spin_unlock(&bdi->wb_lock);

        /*
         * If the default thread isn't there, make sure we add it. When
         * it gets created and wakes up, we'll run this work.
         */
        if (unlikely(!bdi->wb.task)) {
                wake_up_process(default_backing_dev_info.wb.task);
        } else {
                struct bdi_writeback *wb = &bdi->wb;

                if (wb->task)
                        wake_up_process(wb->task);
        }
}

What prevents the flusher task from deciding to commit suicide, set
wb->task to NULL, and make this code to do  wake_up_process(NULL)?

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

             reply	other threads:[~2010-07-07 11:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-07 11:30 Artem Bityutskiy [this message]
2010-07-07 12:28 ` bdi forker Artem Bityutskiy
2010-07-07 15:08 ` Christoph Hellwig

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=1278502227.29620.161.camel@localhost \
    --to=dedekind1@gmail.com \
    --cc=hch@infradead.org \
    --cc=jens.axboe@oracle.com \
    --cc=linux-fsdevel@vger.kernel.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 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.