On Wed, Jul 21, 2010 at 12:31:46PM +0300, Artem Bityutskiy wrote:
> @@ -973,22 +981,37 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> * reposition it (that would break b_dirty time-ordering).
> */
> if (!was_dirty) {
> - struct bdi_writeback *wb = &inode_to_bdi(inode)->wb;
> - struct backing_dev_info *bdi = wb->bdi;
> -
> - if (bdi_cap_writeback_dirty(bdi) &&
> - !test_bit(BDI_registered, &bdi->state)) {
> - WARN_ON(1);
> - printk(KERN_ERR "bdi-%s not registered\n",
> - bdi->name);
> + bdi = inode_to_bdi(inode);
> +
> + if (bdi_cap_writeback_dirty(bdi)) {
> + WARN(!test_bit(BDI_registered, &bdi->state),
> + "bdi-%s not registered\n", bdi->name);
> +
> + /*
> + * If this is the first dirty inode for this
> + * bdi, we have to wake-up the corresponding
> + * bdi thread to make sure background
> + * write-back happens later.
> + */
> + if (!wb_has_dirty_io(&bdi->wb))
> + wakeup_bdi = true;
> }
>
> inode->dirtied_when = jiffies;
> - list_move(&inode->i_list, &wb->b_dirty);
> + list_move(&inode->i_list, &bdi->wb.b_dirty);
> }
> }
> out:
> spin_unlock(&inode_lock);
> +
> + if (wakeup_bdi) {
> + spin_lock(&bdi->wb_lock);
> + if (!bdi->wb.task)
> + wake_up_process(default_backing_dev_info.wb.task);
> + else
> + wake_up_process(bdi->wb.task);
> + spin_unlock(&bdi->wb_lock);
> + }
> }
We really want to wake up the bdi right away when first dirtying
the inode? I haven't looked at where the state of the bdi code is
now, but isn't it better to have a a delay there?
And rather than spreading details of how bdi tasks are managed
would you consider putting this into its own function?
Other than that, I like your patches. Out of interest, is 5 seconds
very detremental to power usage? What is a reasonable goal for
wakeups? (eg. 95%+ of possible efficiency)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
