Re: [PATCH 2/2] jbd2: do not clear the BH_Mapped flag when forgetting a metadata buffer

[Jan Kara <jack@suse.cz>]

On Thu 06-02-20 23:28:01, zhangyi (F) wrote:
> Thanks for the comments.
> 
> On 2020/2/6 19:46, Jan Kara wrote:
> > On Mon 03-02-20 22:04:58, zhangyi (F) wrote:
> [..]
> >> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> >> index 6396fe70085b..a649cdd1c5e5 100644
> >> --- a/fs/jbd2/commit.c
> >> +++ b/fs/jbd2/commit.c
> >> @@ -987,10 +987,13 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> >>  		if (buffer_freed(bh) && !jh->b_next_transaction) {
> >>  			clear_buffer_freed(bh);
> >>  			clear_buffer_jbddirty(bh);
> >> -			clear_buffer_mapped(bh);
> >> -			clear_buffer_new(bh);
> >> -			clear_buffer_req(bh);
> >> -			bh->b_bdev = NULL;
> >> +			if (buffer_unmap(bh)) {
> >> +				clear_buffer_unmap(bh);
> >> +				clear_buffer_mapped(bh);
> >> +				clear_buffer_new(bh);
> >> +				clear_buffer_req(bh);
> >> +				bh->b_bdev = NULL;
> >> +			}
> > 
> > Any reason why you don't want to clear buffer_req and buffer_new flags for
> > all buffers as well? I agree that b_bdev setting and buffer_mapped need
> > special treatment.
> > 
> IIUC, for the buffer coming from jbd2_journal_forget() is always 'block
> device backed' metadata buffer (not pretty sure), and for these metadata
  Yes, it is.

> buffer, buffer_new flag will not be set. At the same time, since it's
> always mapped, so it's fine to keep the buffer_req flag even it's freed
> by the filesystem now, because it means the block device has committed
> this buffer, and it seems that it does not affect we reuse this buffer.
> Am I missing something ?

OK, you're right that buffer_new shouldn't be ever set for block backed
buffers and we don't care about buffer_req. So let's keep the split of bits
to clear as you did and just add a comment that for block device buffers it
is enough to clear buffer_jbddirty and buffer_freed, for file mapping
buffers (i.e., journalled data) we have to be more careful and clear more
bits.

								Honza

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