On Fri, Sep 13, 2019 at 2:54 PM Dennis Zhou <den...@kernel.org> wrote:
>
> Before, if a eb failed to write out, we would end up triggering a
> BUG_ON(). As of f4340622e0226 ("btrfs: extent_io: Move the BUG_ON() in
> flush_write_bio() one level up"), we no longer BUG_ON(), so we should
> make life consistent and add back the unwritten bytes to
> dirty_metadata_bytes.
>
> Signed-off-by: Dennis Zhou <den...@kernel.org>
> Cc: Filipe Manana <fdman...@kernel.org>
Looks good.
However I find the subject very confusing and misleading.
"extent_io read eb to dirty_metadata_bytes on ioerr"
That gives the idea of reading the eb (like from disk? or its content,
reading from its pages?), and the "to dirty_metadata_bytes" also find
it confusing.
Something like:
"btrfs: adjust dirty_metadata_bytes after writeback failure of extent buffer"
would make it clear and not confusing IMHO.
Perhaps it's something David can change when he picks the patch
(either to that or some other more clear subject).
Anyway, for the change itself,
Reviewed-by: Filipe Manana <fdman...@suse.com>
Thanks!
> ---
> fs/btrfs/extent_io.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 1ff438fd5bc2..b67133a23652 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3728,11 +3728,21 @@ static void end_extent_buffer_writeback(struct
> extent_buffer *eb)
> static void set_btree_ioerr(struct page *page)
> {
> struct extent_buffer *eb = (struct extent_buffer *)page->private;
> + struct btrfs_fs_info *fs_info;
>
> SetPageError(page);
> if (test_and_set_bit(EXTENT_BUFFER_WRITE_ERR, &eb->bflags))
> return;
>
> + /*
> + * If we error out, we should add back the dirty_metadata_bytes
> + * to make it consistent.
> + */
> + fs_info = eb->fs_info;
> + percpu_counter_add_batch(&fs_info->dirty_metadata_bytes,
> + eb->len,
> + fs_info->dirty_metadata_batch);
> +
> /*
> * If writeback for a btree extent that doesn't belong to a log tree
> * failed, increment the counter transaction->eb_write_errors.
> --
> 2.17.1
>
