Message ID | 55de39a4.mm24W9YHh8D6r9FS%akpm@linux-foundation.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Wed, Aug 26, 2015 at 03:11:48PM -0700, Andrew Morton wrote: > From: Xue jiufei <xuejiufei@huawei.com> > Subject: ocfs2: extend enough credits for freeing one truncate record while replaying truncate records > > Now function ocfs2_replay_truncate_records() first modifies tl_used, then > calls ocfs2_extend_trans() to extend transactions for gd and alloc inode > used for freeing clusters. jbd2_journal_restart() may be called and it > may happen that tl_used in truncate log is decreased but the clusters are > not freed, which means these clusters are lost. So we should avoid > extending transactions in these two operations. > > Signed-off-by: joyce.xue <xuejiufei@huawei.com> > Cc: Mark Fasheh <mfasheh@suse.com> > Cc: Joel Becker <jlbec@evilplan.org> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > fs/ocfs2/alloc.c | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff -puN fs/ocfs2/alloc.c~extend-enough-credits-for-freeing-one-truncate-record-while-replaying-truncate-records fs/ocfs2/alloc.c > --- a/fs/ocfs2/alloc.c~extend-enough-credits-for-freeing-one-truncate-record-while-replaying-truncate-records > +++ a/fs/ocfs2/alloc.c <looks good> > @@ -6063,7 +6060,7 @@ int __ocfs2_flush_truncate_log(struct oc > goto out_mutex; > } > > - handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE); > + handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_FLUSH_ONE_REC); > if (IS_ERR(handle)) { > status = PTR_ERR(handle); > mlog_errno(status); Why is this particular change here? --Mark -- Mark Fasheh
Hi Mark, On 2015/8/29 7:55, Mark Fasheh wrote: > On Wed, Aug 26, 2015 at 03:11:48PM -0700, Andrew Morton wrote: >> From: Xue jiufei <xuejiufei@huawei.com> >> Subject: ocfs2: extend enough credits for freeing one truncate record while replaying truncate records >> >> Now function ocfs2_replay_truncate_records() first modifies tl_used, then >> calls ocfs2_extend_trans() to extend transactions for gd and alloc inode >> used for freeing clusters. jbd2_journal_restart() may be called and it >> may happen that tl_used in truncate log is decreased but the clusters are >> not freed, which means these clusters are lost. So we should avoid >> extending transactions in these two operations. >> >> Signed-off-by: joyce.xue <xuejiufei@huawei.com> >> Cc: Mark Fasheh <mfasheh@suse.com> >> Cc: Joel Becker <jlbec@evilplan.org> >> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> >> --- >> >> fs/ocfs2/alloc.c | 19 ++++++++----------- >> 1 file changed, 8 insertions(+), 11 deletions(-) >> >> diff -puN fs/ocfs2/alloc.c~extend-enough-credits-for-freeing-one-truncate-record-while-replaying-truncate-records fs/ocfs2/alloc.c >> --- a/fs/ocfs2/alloc.c~extend-enough-credits-for-freeing-one-truncate-record-while-replaying-truncate-records >> +++ a/fs/ocfs2/alloc.c > > <looks good> > >> @@ -6063,7 +6060,7 @@ int __ocfs2_flush_truncate_log(struct oc >> goto out_mutex; >> } >> >> - handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE); >> + handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_FLUSH_ONE_REC); >> if (IS_ERR(handle)) { >> status = PTR_ERR(handle); >> mlog_errno(status); > > Why is this particular change here? > --Mark > I think Joyce wants to get enough credits at first and then call ocfs2_replay_truncate_records. Thanks Joseph > -- > Mark Fasheh > > _______________________________________________ > Ocfs2-devel mailing list > Ocfs2-devel@oss.oracle.com > https://oss.oracle.com/mailman/listinfo/ocfs2-devel > >
On Sat, Aug 29, 2015 at 10:38:09AM +0800, Joseph Qi wrote: > Hi Mark, > > On 2015/8/29 7:55, Mark Fasheh wrote: > > On Wed, Aug 26, 2015 at 03:11:48PM -0700, Andrew Morton wrote: > >> From: Xue jiufei <xuejiufei@huawei.com> > >> Subject: ocfs2: extend enough credits for freeing one truncate record while replaying truncate records > >> > >> Now function ocfs2_replay_truncate_records() first modifies tl_used, then > >> calls ocfs2_extend_trans() to extend transactions for gd and alloc inode > >> used for freeing clusters. jbd2_journal_restart() may be called and it > >> may happen that tl_used in truncate log is decreased but the clusters are > >> not freed, which means these clusters are lost. So we should avoid > >> extending transactions in these two operations. > >> > >> Signed-off-by: joyce.xue <xuejiufei@huawei.com> > >> Cc: Mark Fasheh <mfasheh@suse.com> > >> Cc: Joel Becker <jlbec@evilplan.org> > >> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > >> --- > >> > >> fs/ocfs2/alloc.c | 19 ++++++++----------- > >> 1 file changed, 8 insertions(+), 11 deletions(-) > >> > >> diff -puN fs/ocfs2/alloc.c~extend-enough-credits-for-freeing-one-truncate-record-while-replaying-truncate-records fs/ocfs2/alloc.c > >> --- a/fs/ocfs2/alloc.c~extend-enough-credits-for-freeing-one-truncate-record-while-replaying-truncate-records > >> +++ a/fs/ocfs2/alloc.c > > > > <looks good> > > > >> @@ -6063,7 +6060,7 @@ int __ocfs2_flush_truncate_log(struct oc > >> goto out_mutex; > >> } > >> > >> - handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE); > >> + handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_FLUSH_ONE_REC); > >> if (IS_ERR(handle)) { > >> status = PTR_ERR(handle); > >> mlog_errno(status); > > > > Why is this particular change here? > > --Mark > > > I think Joyce wants to get enough credits at first and then call > ocfs2_replay_truncate_records. Oh ok I see. That's fine then, the patch otherwise looked good to me, thanks. Reviewed-by: Mark Fasheh <mfasheh@suse.de> --Mark -- Mark Fasheh
On Tue, 1 Sep 2015 10:54:03 -0700 Mark Fasheh <mfasheh@suse.de> wrote: > > > Why is this particular change here? > > > --Mark > > > > > I think Joyce wants to get enough credits at first and then call > > ocfs2_replay_truncate_records. > > Oh ok I see. That's fine then, the patch otherwise looked good to me, > thanks. > > Reviewed-by: Mark Fasheh <mfasheh@suse.de> Cool. And where are we at with http://ozlabs.org/~akpm/mmots/broken-out/ocfs2-extend-transaction-for-ocfs2_remove_rightmost_path-and-ocfs2_update_edge_lengths-before-to-avoid-inconsistency-between-inode-and-et.patch ?
On Tue, Sep 01, 2015 at 03:12:07PM -0700, Andrew Morton wrote: > On Tue, 1 Sep 2015 10:54:03 -0700 Mark Fasheh <mfasheh@suse.de> wrote: > > > > > Why is this particular change here? > > > > --Mark > > > > > > > I think Joyce wants to get enough credits at first and then call > > > ocfs2_replay_truncate_records. > > > > Oh ok I see. That's fine then, the patch otherwise looked good to me, > > thanks. > > > > Reviewed-by: Mark Fasheh <mfasheh@suse.de> > > Cool. And where are we at with > http://ozlabs.org/~akpm/mmots/broken-out/ocfs2-extend-transaction-for-ocfs2_remove_rightmost_path-and-ocfs2_update_edge_lengths-before-to-avoid-inconsistency-between-inode-and-et.patch > > ? I had a couple questions, we're waiting for a response: https://www.mail-archive.com/ocfs2-devel@oss.oracle.com/msg10167.html --Mark -- Mark Fasheh
diff -puN fs/ocfs2/alloc.c~extend-enough-credits-for-freeing-one-truncate-record-while-replaying-truncate-records fs/ocfs2/alloc.c --- a/fs/ocfs2/alloc.c~extend-enough-credits-for-freeing-one-truncate-record-while-replaying-truncate-records +++ a/fs/ocfs2/alloc.c @@ -5975,16 +5975,6 @@ static int ocfs2_replay_truncate_records ocfs2_journal_dirty(handle, tl_bh); - /* TODO: Perhaps we can calculate the bulk of the - * credits up front rather than extending like - * this. */ - status = ocfs2_extend_trans(handle, - OCFS2_TRUNCATE_LOG_FLUSH_ONE_REC); - if (status < 0) { - mlog_errno(status); - goto bail; - } - rec = tl->tl_recs[i]; start_blk = ocfs2_clusters_to_blocks(data_alloc_inode->i_sb, le32_to_cpu(rec.t_start)); @@ -6005,6 +5995,13 @@ static int ocfs2_replay_truncate_records goto bail; } } + + status = ocfs2_extend_trans(handle, + OCFS2_TRUNCATE_LOG_FLUSH_ONE_REC); + if (status < 0) { + mlog_errno(status); + goto bail; + } i--; } @@ -6063,7 +6060,7 @@ int __ocfs2_flush_truncate_log(struct oc goto out_mutex; } - handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_UPDATE); + handle = ocfs2_start_trans(osb, OCFS2_TRUNCATE_LOG_FLUSH_ONE_REC); if (IS_ERR(handle)) { status = PTR_ERR(handle); mlog_errno(status);