[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181025114156.GB7711@quack2.suse.cz>
Date: Thu, 25 Oct 2018 13:41:56 +0200
From: Jan Kara <jack@...e.cz>
To: Fisher <fisherthepooh@...tonmail.com>
Cc: "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: Why not always do copy-out in jbd2_journal_write_metadta_buffer
Hi,
On Mon 22-10-18 10:34:48, Fisher wrote:
> Recently I was testing my storage's performance and found that there were
> periodic performance drops when I ran sequential write benchmark.
> After profiling the duration in each step, I found that the dropping
> performance was due to wait_on_bit_io(&bh->b_state, BH_Shadow,
> TASK_UNINTERRUPTIBLE) in do_get_write_access(). And this is what made me
> confused, if my understanding was right, I thought buffer_shadow(bh)
> stands for buffer not being copied-out that's why we should wait.
No buffer_shadow() just means that some version of the buffer is being
written to the journal.
> But why don't we do copy-out in jbd2_journal_write_metadta_buffer()? and
> if we do do the copy-out, does that mean we don't have to
> set_buffer_shadow because it refers to buffer not copied-out?
The code in do_get_write_access() checks whether the buffer already has
frozen data (by checking jh->b_frozen_data) and if not, if it is just being
written to the journal (buffer_shadow() check in there). If it is, we have
to wait because we cannot modify the version of the buffer that goes to the
journal.
> I made a test, when a buffer_head goes into
> jbd2_journal_write_metadta_buffer(), as long as it belongs to metadata,
> then force it to do copy-out and do not set_buffer_shadow, then there
> will be no periodic performance drops. Is this test reasonable?
Well, it does trade of CPU overhead (you know copy buffer for each
transaction) for the latency (no need to wait for buffer writeout when
redirtying buffer). Usually I wouldn't consider this worth it but obviously
it depends on the workload...
Honza
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists