答复: Why not always do copy-out in jbd2_journal_write_metadta_buffer

["王小光(费曼)" <feiman.wxg@alibaba-inc.com>]

Hi jan,

>
>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...
I also run into this issue.
In do_get_write_access(), there are such comments:
/*
	 * There is one case we have to be very careful about.  If the
	 * committing transaction is currently writing this buffer out to
disk
	 * and has NOT made a copy-out, then we cannot modify the buffer
	 * contents at all right now.  The essence of copy-out is that it is
	 * the extra copy, not the primary copy, which gets journaled.  If
the
	 * primary copy is already going to disk then we cannot do copy-out
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      I wonder why we can not do copy-out here? We can not visit this buffer
while it's
      going to disk? 

	 * here.
	 */
	if (buffer_shadow(bh)) {
		JBUFFER_TRACE(jh, "on shadow: sleep");
		jbd_unlock_bh_state(bh);
		wait_on_bit_io(&bh->b_state, BH_Shadow,
TASK_UNINTERRUPTIBLE);
		goto repeat;
	}

Regards,
Xiaoguang Wang


>
>								Honza
>--
>Jan Kara <jack@suse.com>
>SUSE Labs, CR