On Tue 11-02-20 21:54:59, zhangyi (F) wrote:
> There is no need to delay the clearing of b_modified flag to the
> transaction committing time when unmapping the journalled buffer, so
> just move it to the journal_unmap_buffer().
>
> Signed-off-by: zhangyi (F) <yi.zhang@xxxxxxxxxx>
The patch looks good to me. You can add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/jbd2/commit.c | 43 +++++++++++++++----------------------------
> fs/jbd2/transaction.c | 10 ++++++----
> 2 files changed, 21 insertions(+), 32 deletions(-)
>
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 7f0b362b3842..ecc2ea5f1b59 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -976,34 +976,21 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> * it. */
>
> /*
> - * A buffer which has been freed while still being journaled by
> - * a previous transaction.
> - */
> - if (buffer_freed(bh)) {
> - /*
> - * If the running transaction is the one containing
> - * "add to orphan" operation (b_next_transaction !=
> - * NULL), we have to wait for that transaction to
> - * commit before we can really get rid of the buffer.
> - * So just clear b_modified to not confuse transaction
> - * credit accounting and refile the buffer to
> - * BJ_Forget of the running transaction. If the just
> - * committed transaction contains "add to orphan"
> - * operation, we can completely invalidate the buffer
> - * now. We are rather through in that since the
> - * buffer may be still accessible when blocksize <
> - * pagesize and it is attached to the last partial
> - * page.
> - */
> - jh->b_modified = 0;
> - if (!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;
> - }
> + * A buffer which has been freed while still being journaled
> + * by a previous transaction, refile the buffer to BJ_Forget of
> + * the running transaction. If the just committed transaction
> + * contains "add to orphan" operation, we can completely
> + * invalidate the buffer now. We are rather through in that
> + * since the buffer may be still accessible when blocksize <
> + * pagesize and it is attached to the last partial page.
> + */
> + 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_jbddirty(bh)) {
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 27b9f9dee434..0603dfa9ad90 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -2329,14 +2329,16 @@ static int journal_unmap_buffer(journal_t *journal, struct buffer_head *bh,
> return -EBUSY;
> }
> /*
> - * OK, buffer won't be reachable after truncate. We just set
> - * j_next_transaction to the running transaction (if there is
> - * one) and mark buffer as freed so that commit code knows it
> - * should clear dirty bits when it is done with the buffer.
> + * OK, buffer won't be reachable after truncate. We just clear
> + * b_modified to not confuse transaction credit accounting, and
> + * set j_next_transaction to the running transaction (if there
> + * is one) and mark buffer as freed so that commit code knows
> + * it should clear dirty bits when it is done with the buffer.
> */
> set_buffer_freed(bh);
> if (journal->j_running_transaction && buffer_jbddirty(bh))
> jh->b_next_transaction = journal->j_running_transaction;
> + jh->b_modified = 0;
> spin_unlock(&journal->j_list_lock);
> spin_unlock(&jh->b_state_lock);
> write_unlock(&journal->j_state_lock);
> --
> 2.17.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
