Hello, Josef.

On Tue, Oct 25, 2016 at 02:41:41PM -0400, Josef Bacik wrote:
> These are counters that constantly go up in order to do bandwidth 
> calculations.
> It isn't important what the units are in, as long as they are consistent 
> between
> the two of them, so convert them to count bytes written/dirtied, and allow the
> metadata accounting stuff to change the counters as well.
> 
> Signed-off-by: Josef Bacik <jba...@fb.com>

Acked-by: Tejun Heo <t...@kernel.org>

A small nit below.

> @@ -2547,12 +2547,16 @@ void account_page_redirty(struct page *page)
>       if (mapping && mapping_cap_account_dirty(mapping)) {
>               struct inode *inode = mapping->host;
>               struct bdi_writeback *wb;
> +             unsigned long flags;
>               bool locked;
>  
>               wb = unlocked_inode_to_wb_begin(inode, &locked);
>               current->nr_dirtied--;
> -             dec_node_page_state(page, NR_DIRTIED);
> -             dec_wb_stat(wb, WB_DIRTIED);
> +
> +             local_irq_save(flags);
> +             __dec_node_page_state(page, NR_DIRTIED);
> +             __add_wb_stat(wb, WB_DIRTIED_BYTES, -(long)PAGE_SIZE);
> +             local_irq_restore(flags);

Hmmm... so, the explicit irq clustering is neutral or win as the code
currently stands but AFAICS that's just because add_wb_stat() doesn't
use the right percpu ops.  If we convert add_wb_stat() to use the
matching percpu ops, the above change would be more expensive at least
on x86.  Maybe just skip this part?

Thanks.

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

Reply via email to