* [Ocfs2-devel] [PATCH] Ocfs2: Rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed @ 2014-03-31 13:07 Younger Liu 2014-03-31 21:02 ` Andrew Morton 2014-04-01 17:48 ` Mark Fasheh 0 siblings, 2 replies; 4+ messages in thread From: Younger Liu @ 2014-03-31 13:07 UTC (permalink / raw) To: ocfs2-devel After updating alloc_dinode counts in ocfs2_alloc_dinode_update_counts(), if ocfs2_alloc_dinode_update_bitmap() failed, there is a rare case that some space may be lost. So, we rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed. Signed-off-by: Younger Liu <younger.liucn@gmail.com> Cc: Mark Fasheh <mfasheh@suse.de> --- fs/ocfs2/move_extents.c | 5 ++++- fs/ocfs2/suballoc.c | 24 +++++++++++++++++++++++- fs/ocfs2/suballoc.h | 4 ++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c index 64c304d..65483d0 100644 --- a/fs/ocfs2/move_extents.c +++ b/fs/ocfs2/move_extents.c @@ -690,8 +690,11 @@ static int ocfs2_move_extent(struct ocfs2_move_extents_context *context, ret = ocfs2_block_group_set_bits(handle, gb_inode, gd, gd_bh, goal_bit, len); - if (ret) + if (ret) { + ocfs2_rollback_alloc_dinode_counts(gb_inode, gb_bh, len, + le16_to_cpu(gd->bg_chain)); mlog_errno(ret); + } /* * Here we should write the new page out first if we are diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c index 47ae266..150d94f 100644 --- a/fs/ocfs2/suballoc.c +++ b/fs/ocfs2/suballoc.c @@ -1607,6 +1607,20 @@ out: return ret; } +void ocfs2_rollback_alloc_dinode_counts(struct inode *inode, + struct buffer_head *di_bh, + u32 num_bits, + u16 chain) +{ + u32 tmp_used; + struct ocfs2_dinode *di = (struct ocfs2_dinode *) di_bh->b_data; + struct ocfs2_chain_list *cl = (struct ocfs2_chain_list *) &di->id2.i_chain; + + tmp_used = le32_to_cpu(di->id1.bitmap1.i_used); + di->id1.bitmap1.i_used = cpu_to_le32(tmp_used - num_bits); + le32_add_cpu(&cl->cl_recs[chain].c_free, num_bits); +} + static int ocfs2_bg_discontig_fix_by_rec(struct ocfs2_suballoc_result *res, struct ocfs2_extent_rec *rec, struct ocfs2_chain_list *cl) @@ -1707,8 +1721,12 @@ static int ocfs2_search_one_group(struct ocfs2_alloc_context *ac, ret = ocfs2_block_group_set_bits(handle, alloc_inode, gd, group_bh, res->sr_bit_offset, res->sr_bits); - if (ret < 0) + if (ret < 0) { + ocfs2_rollback_alloc_dinode_counts(alloc_inode, ac->ac_bh, + res->sr_bits, + le16_to_cpu(gd->bg_chain)); mlog_errno(ret); + } out_loc_only: *bits_left = le16_to_cpu(gd->bg_free_bits_count); @@ -1838,6 +1856,8 @@ static int ocfs2_search_chain(struct ocfs2_alloc_context *ac, res->sr_bit_offset, res->sr_bits); if (status < 0) { + ocfs2_rollback_alloc_dinode_counts(alloc_inode, + ac->ac_bh, res->sr_bits, chain); mlog_errno(status); goto bail; } @@ -2149,6 +2169,8 @@ int ocfs2_claim_new_inode_at_loc(handle_t *handle, res->sr_bit_offset, res->sr_bits); if (ret < 0) { + ocfs2_rollback_alloc_dinode_counts(ac->ac_inode, + ac->ac_bh, res->sr_bits, chain); mlog_errno(ret); goto out; } diff --git a/fs/ocfs2/suballoc.h b/fs/ocfs2/suballoc.h index 218d803..2d25017 100644 --- a/fs/ocfs2/suballoc.h +++ b/fs/ocfs2/suballoc.h @@ -91,6 +91,10 @@ int ocfs2_alloc_dinode_update_counts(struct inode *inode, struct buffer_head *di_bh, u32 num_bits, u16 chain); +void ocfs2_rollback_alloc_dinode_counts(struct inode *inode, + struct buffer_head *di_bh, + u32 num_bits, + u16 chain); int ocfs2_block_group_set_bits(handle_t *handle, struct inode *alloc_inode, struct ocfs2_group_desc *bg, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [PATCH] Ocfs2: Rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed 2014-03-31 13:07 [Ocfs2-devel] [PATCH] Ocfs2: Rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed Younger Liu @ 2014-03-31 21:02 ` Andrew Morton 2014-04-01 0:37 ` Younger Liu 2014-04-01 17:48 ` Mark Fasheh 1 sibling, 1 reply; 4+ messages in thread From: Andrew Morton @ 2014-03-31 21:02 UTC (permalink / raw) To: ocfs2-devel On Mon, 31 Mar 2014 21:07:40 +0800 Younger Liu <younger.liucn@gmail.com> wrote: > After updating alloc_dinode counts in ocfs2_alloc_dinode_update_counts(), > if ocfs2_alloc_dinode_update_bitmap() failed, there is a rare case that some > space may be lost. > So, we rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed. This patch wildly conflicts with your earlier patch ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously.patch. below. What should be done? From: Younger Liu <younger.liucn@gmail.com> Subject: ocfs2: alloc_dinode counts and group bitmap should be update simultaneously Updating alloc_dinode counts in ocfs2_alloc_dinode_update_counts() and setting group bitmap in ocfs2_alloc_dinode_update_bitmap() have to be done simultaneously. Two cases are as follow: (1) If ocfs2_alloc_dinode_update_counts() fails, there is no need to set group bitmap. This case has been considered. (2) If ocfs2_alloc_dinode_update_bitmap() fails, alloc_dinode counts should be rolled back. Otherwise, some clusters would never be used. This case is not considered. So, we combine two functions, and ensure simultaneity. Signed-off-by: Younger Liu <younger.liucn@gmail.com> Cc: Joel Becker <jlbec@evilplan.org> Cc: Mark Fasheh <mfasheh@suse.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- fs/ocfs2/move_extents.c | 11 -- fs/ocfs2/ocfs2_trace.h | 2 fs/ocfs2/suballoc.c | 155 +++++++++++++++----------------------- fs/ocfs2/suballoc.h | 21 +---- 4 files changed, 77 insertions(+), 112 deletions(-) diff -puN fs/ocfs2/move_extents.c~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously fs/ocfs2/move_extents.c --- a/fs/ocfs2/move_extents.c~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously +++ a/fs/ocfs2/move_extents.c @@ -681,18 +681,15 @@ static int ocfs2_move_extent(struct ocfs } gd = (struct ocfs2_group_desc *)gd_bh->b_data; - ret = ocfs2_alloc_dinode_update_counts(gb_inode, handle, gb_bh, len, - le16_to_cpu(gd->bg_chain)); + ret = ocfs2_alloc_dinode_update_bitmap(handle, + gb_inode, gb_bh, gd, gb_bh, + le16_to_cpu(gd->bg_chain), + goal_bit, len); if (ret) { mlog_errno(ret); goto out_commit; } - ret = ocfs2_block_group_set_bits(handle, gb_inode, gd, gd_bh, - goal_bit, len); - if (ret) - mlog_errno(ret); - /* * Here we should write the new page out first if we are * in write-back mode. diff -puN fs/ocfs2/ocfs2_trace.h~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously fs/ocfs2/ocfs2_trace.h --- a/fs/ocfs2/ocfs2_trace.h~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously +++ a/fs/ocfs2/ocfs2_trace.h @@ -788,7 +788,7 @@ DEFINE_OCFS2_UINT_UINT_UINT_EVENT(ocfs2_ DEFINE_OCFS2_ULL_EVENT(ocfs2_reserve_new_inode_new_group); -DEFINE_OCFS2_UINT_UINT_EVENT(ocfs2_block_group_set_bits); +DEFINE_OCFS2_UINT_UINT_EVENT(ocfs2_alloc_dinode_update_bitmap); TRACE_EVENT(ocfs2_relink_block_group, TP_PROTO(unsigned long long i_blkno, unsigned int chain, diff -puN fs/ocfs2/suballoc.c~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously fs/ocfs2/suballoc.c --- a/fs/ocfs2/suballoc.c~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously +++ a/fs/ocfs2/suballoc.c @@ -1337,54 +1337,6 @@ static int ocfs2_block_group_find_clear_ return status; } -int ocfs2_block_group_set_bits(handle_t *handle, - struct inode *alloc_inode, - struct ocfs2_group_desc *bg, - struct buffer_head *group_bh, - unsigned int bit_off, - unsigned int num_bits) -{ - int status; - void *bitmap = bg->bg_bitmap; - int journal_type = OCFS2_JOURNAL_ACCESS_WRITE; - - /* All callers get the descriptor via - * ocfs2_read_group_descriptor(). Any corruption is a code bug. */ - BUG_ON(!OCFS2_IS_VALID_GROUP_DESC(bg)); - BUG_ON(le16_to_cpu(bg->bg_free_bits_count) < num_bits); - - trace_ocfs2_block_group_set_bits(bit_off, num_bits); - - if (ocfs2_is_cluster_bitmap(alloc_inode)) - journal_type = OCFS2_JOURNAL_ACCESS_UNDO; - - status = ocfs2_journal_access_gd(handle, - INODE_CACHE(alloc_inode), - group_bh, - journal_type); - if (status < 0) { - mlog_errno(status); - goto bail; - } - - le16_add_cpu(&bg->bg_free_bits_count, -num_bits); - if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) { - ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" - " count %u but claims %u are freed. num_bits %d", - (unsigned long long)le64_to_cpu(bg->bg_blkno), - le16_to_cpu(bg->bg_bits), - le16_to_cpu(bg->bg_free_bits_count), num_bits); - return -EROFS; - } - while(num_bits--) - ocfs2_set_bit(bit_off++, bitmap); - - ocfs2_journal_dirty(handle, group_bh); - -bail: - return status; -} - /* find the one with the most empty bits */ static inline u16 ocfs2_find_victim_chain(struct ocfs2_chain_list *cl) { @@ -1580,31 +1532,78 @@ static int ocfs2_block_group_search(stru return ret; } -int ocfs2_alloc_dinode_update_counts(struct inode *inode, - handle_t *handle, - struct buffer_head *di_bh, - u32 num_bits, - u16 chain) +int ocfs2_alloc_dinode_update_bitmap(handle_t *handle, + struct inode *alloc_inode, + struct buffer_head *di_bh, + struct ocfs2_group_desc *bg, + struct buffer_head *group_bh, + u16 chain, u32 bit_off, u32 num_bits) { int ret; u32 tmp_used; struct ocfs2_dinode *di = (struct ocfs2_dinode *) di_bh->b_data; struct ocfs2_chain_list *cl = (struct ocfs2_chain_list *) &di->id2.i_chain; + void *bitmap = bg->bg_bitmap; + int journal_type = OCFS2_JOURNAL_ACCESS_WRITE; - ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh, - OCFS2_JOURNAL_ACCESS_WRITE); + /* + * All callers get the descriptor via + * ocfs2_read_group_descriptor(). Any corruption is a code bug. + */ + BUG_ON(!OCFS2_IS_VALID_GROUP_DESC(bg)); + BUG_ON(le16_to_cpu(bg->bg_free_bits_count) < num_bits); + + trace_ocfs2_alloc_dinode_update_bitmap(bit_off, num_bits); + + ret = ocfs2_journal_access_di(handle, + INODE_CACHE(alloc_inode), di_bh, journal_type); if (ret < 0) { mlog_errno(ret); goto out; } + if (ocfs2_is_cluster_bitmap(alloc_inode)) + journal_type = OCFS2_JOURNAL_ACCESS_UNDO; + + ret = ocfs2_journal_access_gd(handle, + INODE_CACHE(alloc_inode), group_bh, journal_type); + if (ret < 0) { + mlog_errno(ret); + goto out; + } + + /* update alloc_dinode counts */ tmp_used = le32_to_cpu(di->id1.bitmap1.i_used); di->id1.bitmap1.i_used = cpu_to_le32(num_bits + tmp_used); le32_add_cpu(&cl->cl_recs[chain].c_free, -num_bits); + + /* update bg counts and bitmap*/ + le16_add_cpu(&bg->bg_free_bits_count, -num_bits); + if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) { + ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" + " count %u but claims %u are freed. num_bits %d", + (unsigned long long)le64_to_cpu(bg->bg_blkno), + le16_to_cpu(bg->bg_bits), + le16_to_cpu(bg->bg_free_bits_count), num_bits); + ret = -EROFS; + goto out_rollback; + } + while (num_bits--) + ocfs2_set_bit(bit_off++, bitmap); + ocfs2_journal_dirty(handle, di_bh); + ocfs2_journal_dirty(handle, group_bh); out: return ret; + +out_rollback: + le16_add_cpu(&bg->bg_free_bits_count, num_bits); + + di->id1.bitmap1.i_used = cpu_to_le32(tmp_used - num_bits); + le32_add_cpu(&cl->cl_recs[chain].c_free, num_bits); + + return ret; } static int ocfs2_bg_discontig_fix_by_rec(struct ocfs2_suballoc_result *res, @@ -1697,19 +1696,15 @@ static int ocfs2_search_one_group(struct if (ac->ac_find_loc_only) goto out_loc_only; - ret = ocfs2_alloc_dinode_update_counts(alloc_inode, handle, ac->ac_bh, - res->sr_bits, - le16_to_cpu(gd->bg_chain)); + ret = ocfs2_alloc_dinode_update_bitmap(handle, + alloc_inode, ac->ac_bh, gd, group_bh, + le16_to_cpu(gd->bg_chain), + res->sr_bit_offset, res->sr_bits); if (ret < 0) { mlog_errno(ret); goto out; } - ret = ocfs2_block_group_set_bits(handle, alloc_inode, gd, group_bh, - res->sr_bit_offset, res->sr_bits); - if (ret < 0) - mlog_errno(ret); - out_loc_only: *bits_left = le16_to_cpu(gd->bg_free_bits_count); @@ -1823,20 +1818,9 @@ static int ocfs2_search_chain(struct ocf if (ac->ac_find_loc_only) goto out_loc_only; - status = ocfs2_alloc_dinode_update_counts(alloc_inode, handle, - ac->ac_bh, res->sr_bits, - chain); - if (status) { - mlog_errno(status); - goto bail; - } - - status = ocfs2_block_group_set_bits(handle, - alloc_inode, - bg, - group_bh, - res->sr_bit_offset, - res->sr_bits); + status = ocfs2_alloc_dinode_update_bitmap(handle, + alloc_inode, ac->ac_bh, bg, group_bh, + chain, res->sr_bit_offset, res->sr_bits); if (status < 0) { mlog_errno(status); goto bail; @@ -2134,20 +2118,9 @@ int ocfs2_claim_new_inode_at_loc(handle_ bg = (struct ocfs2_group_desc *) bg_bh->b_data; chain = le16_to_cpu(bg->bg_chain); - ret = ocfs2_alloc_dinode_update_counts(ac->ac_inode, handle, - ac->ac_bh, res->sr_bits, - chain); - if (ret) { - mlog_errno(ret); - goto out; - } - - ret = ocfs2_block_group_set_bits(handle, - ac->ac_inode, - bg, - bg_bh, - res->sr_bit_offset, - res->sr_bits); + ret = ocfs2_alloc_dinode_update_bitmap(handle, + ac->ac_inode, ac->ac_bh, bg, bg_bh, + chain, res->sr_bit_offset, res->sr_bits); if (ret < 0) { mlog_errno(ret); goto out; diff -puN fs/ocfs2/suballoc.h~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously fs/ocfs2/suballoc.h --- a/fs/ocfs2/suballoc.h~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously +++ a/fs/ocfs2/suballoc.h @@ -85,19 +85,14 @@ int ocfs2_reserve_new_inode(struct ocfs2 int ocfs2_reserve_clusters(struct ocfs2_super *osb, u32 bits_wanted, struct ocfs2_alloc_context **ac); - -int ocfs2_alloc_dinode_update_counts(struct inode *inode, - handle_t *handle, - struct buffer_head *di_bh, - u32 num_bits, - u16 chain); -int ocfs2_block_group_set_bits(handle_t *handle, - struct inode *alloc_inode, - struct ocfs2_group_desc *bg, - struct buffer_head *group_bh, - unsigned int bit_off, - unsigned int num_bits); - +int ocfs2_alloc_dinode_update_bitmap(handle_t *handle, + struct inode *alloc_inode, + struct buffer_head *di_bh, + struct ocfs2_group_desc *bg, + struct buffer_head *group_bh, + u16 chain, + u32 bit_off, + u32 num_bits); int ocfs2_claim_metadata(handle_t *handle, struct ocfs2_alloc_context *ac, u32 bits_wanted, _ ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [PATCH] Ocfs2: Rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed 2014-03-31 21:02 ` Andrew Morton @ 2014-04-01 0:37 ` Younger Liu 0 siblings, 0 replies; 4+ messages in thread From: Younger Liu @ 2014-04-01 0:37 UTC (permalink / raw) To: ocfs2-devel On 2014/4/1 5:02, Andrew Morton wrote: > On Mon, 31 Mar 2014 21:07:40 +0800 Younger Liu <younger.liucn@gmail.com> wrote: > >> After updating alloc_dinode counts in ocfs2_alloc_dinode_update_counts(), >> if ocfs2_alloc_dinode_update_bitmap() failed, there is a rare case that some >> space may be lost. >> So, we rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed. > > This patch wildly conflicts with your earlier patch > ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously.patch. > below. > > What should be done? I am sorry, about the earlier patch ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously.patch. I forget to tell you drop it from mm-tree. Now, I follow the suggestion from Mark, and fix the same risk with a new approach: Ocfs2-Rollback-dinode-counts-when-ocfs2_block_group_set_bits-failed.patch Younger > > > From: Younger Liu <younger.liucn@gmail.com> > Subject: ocfs2: alloc_dinode counts and group bitmap should be update simultaneously > > Updating alloc_dinode counts in ocfs2_alloc_dinode_update_counts() and > setting group bitmap in ocfs2_alloc_dinode_update_bitmap() have to be done > simultaneously. Two cases are as follow: > > (1) If ocfs2_alloc_dinode_update_counts() fails, there is no need to > set group bitmap. This case has been considered. > > (2) If ocfs2_alloc_dinode_update_bitmap() fails, alloc_dinode counts > should be rolled back. Otherwise, some clusters would never be used. > This case is not considered. > > So, we combine two functions, and ensure simultaneity. > > Signed-off-by: Younger Liu <younger.liucn@gmail.com> > Cc: Joel Becker <jlbec@evilplan.org> > Cc: Mark Fasheh <mfasheh@suse.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > fs/ocfs2/move_extents.c | 11 -- > fs/ocfs2/ocfs2_trace.h | 2 > fs/ocfs2/suballoc.c | 155 +++++++++++++++----------------------- > fs/ocfs2/suballoc.h | 21 +---- > 4 files changed, 77 insertions(+), 112 deletions(-) > > diff -puN fs/ocfs2/move_extents.c~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously fs/ocfs2/move_extents.c > --- a/fs/ocfs2/move_extents.c~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously > +++ a/fs/ocfs2/move_extents.c > @@ -681,18 +681,15 @@ static int ocfs2_move_extent(struct ocfs > } > > gd = (struct ocfs2_group_desc *)gd_bh->b_data; > - ret = ocfs2_alloc_dinode_update_counts(gb_inode, handle, gb_bh, len, > - le16_to_cpu(gd->bg_chain)); > + ret = ocfs2_alloc_dinode_update_bitmap(handle, > + gb_inode, gb_bh, gd, gb_bh, > + le16_to_cpu(gd->bg_chain), > + goal_bit, len); > if (ret) { > mlog_errno(ret); > goto out_commit; > } > > - ret = ocfs2_block_group_set_bits(handle, gb_inode, gd, gd_bh, > - goal_bit, len); > - if (ret) > - mlog_errno(ret); > - > /* > * Here we should write the new page out first if we are > * in write-back mode. > diff -puN fs/ocfs2/ocfs2_trace.h~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously fs/ocfs2/ocfs2_trace.h > --- a/fs/ocfs2/ocfs2_trace.h~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously > +++ a/fs/ocfs2/ocfs2_trace.h > @@ -788,7 +788,7 @@ DEFINE_OCFS2_UINT_UINT_UINT_EVENT(ocfs2_ > > DEFINE_OCFS2_ULL_EVENT(ocfs2_reserve_new_inode_new_group); > > -DEFINE_OCFS2_UINT_UINT_EVENT(ocfs2_block_group_set_bits); > +DEFINE_OCFS2_UINT_UINT_EVENT(ocfs2_alloc_dinode_update_bitmap); > > TRACE_EVENT(ocfs2_relink_block_group, > TP_PROTO(unsigned long long i_blkno, unsigned int chain, > diff -puN fs/ocfs2/suballoc.c~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously fs/ocfs2/suballoc.c > --- a/fs/ocfs2/suballoc.c~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously > +++ a/fs/ocfs2/suballoc.c > @@ -1337,54 +1337,6 @@ static int ocfs2_block_group_find_clear_ > return status; > } > > -int ocfs2_block_group_set_bits(handle_t *handle, > - struct inode *alloc_inode, > - struct ocfs2_group_desc *bg, > - struct buffer_head *group_bh, > - unsigned int bit_off, > - unsigned int num_bits) > -{ > - int status; > - void *bitmap = bg->bg_bitmap; > - int journal_type = OCFS2_JOURNAL_ACCESS_WRITE; > - > - /* All callers get the descriptor via > - * ocfs2_read_group_descriptor(). Any corruption is a code bug. */ > - BUG_ON(!OCFS2_IS_VALID_GROUP_DESC(bg)); > - BUG_ON(le16_to_cpu(bg->bg_free_bits_count) < num_bits); > - > - trace_ocfs2_block_group_set_bits(bit_off, num_bits); > - > - if (ocfs2_is_cluster_bitmap(alloc_inode)) > - journal_type = OCFS2_JOURNAL_ACCESS_UNDO; > - > - status = ocfs2_journal_access_gd(handle, > - INODE_CACHE(alloc_inode), > - group_bh, > - journal_type); > - if (status < 0) { > - mlog_errno(status); > - goto bail; > - } > - > - le16_add_cpu(&bg->bg_free_bits_count, -num_bits); > - if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) { > - ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" > - " count %u but claims %u are freed. num_bits %d", > - (unsigned long long)le64_to_cpu(bg->bg_blkno), > - le16_to_cpu(bg->bg_bits), > - le16_to_cpu(bg->bg_free_bits_count), num_bits); > - return -EROFS; > - } > - while(num_bits--) > - ocfs2_set_bit(bit_off++, bitmap); > - > - ocfs2_journal_dirty(handle, group_bh); > - > -bail: > - return status; > -} > - > /* find the one with the most empty bits */ > static inline u16 ocfs2_find_victim_chain(struct ocfs2_chain_list *cl) > { > @@ -1580,31 +1532,78 @@ static int ocfs2_block_group_search(stru > return ret; > } > > -int ocfs2_alloc_dinode_update_counts(struct inode *inode, > - handle_t *handle, > - struct buffer_head *di_bh, > - u32 num_bits, > - u16 chain) > +int ocfs2_alloc_dinode_update_bitmap(handle_t *handle, > + struct inode *alloc_inode, > + struct buffer_head *di_bh, > + struct ocfs2_group_desc *bg, > + struct buffer_head *group_bh, > + u16 chain, u32 bit_off, u32 num_bits) > { > int ret; > u32 tmp_used; > struct ocfs2_dinode *di = (struct ocfs2_dinode *) di_bh->b_data; > struct ocfs2_chain_list *cl = (struct ocfs2_chain_list *) &di->id2.i_chain; > + void *bitmap = bg->bg_bitmap; > + int journal_type = OCFS2_JOURNAL_ACCESS_WRITE; > > - ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), di_bh, > - OCFS2_JOURNAL_ACCESS_WRITE); > + /* > + * All callers get the descriptor via > + * ocfs2_read_group_descriptor(). Any corruption is a code bug. > + */ > + BUG_ON(!OCFS2_IS_VALID_GROUP_DESC(bg)); > + BUG_ON(le16_to_cpu(bg->bg_free_bits_count) < num_bits); > + > + trace_ocfs2_alloc_dinode_update_bitmap(bit_off, num_bits); > + > + ret = ocfs2_journal_access_di(handle, > + INODE_CACHE(alloc_inode), di_bh, journal_type); > if (ret < 0) { > mlog_errno(ret); > goto out; > } > > + if (ocfs2_is_cluster_bitmap(alloc_inode)) > + journal_type = OCFS2_JOURNAL_ACCESS_UNDO; > + > + ret = ocfs2_journal_access_gd(handle, > + INODE_CACHE(alloc_inode), group_bh, journal_type); > + if (ret < 0) { > + mlog_errno(ret); > + goto out; > + } > + > + /* update alloc_dinode counts */ > tmp_used = le32_to_cpu(di->id1.bitmap1.i_used); > di->id1.bitmap1.i_used = cpu_to_le32(num_bits + tmp_used); > le32_add_cpu(&cl->cl_recs[chain].c_free, -num_bits); > + > + /* update bg counts and bitmap*/ > + le16_add_cpu(&bg->bg_free_bits_count, -num_bits); > + if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) { > + ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" > + " count %u but claims %u are freed. num_bits %d", > + (unsigned long long)le64_to_cpu(bg->bg_blkno), > + le16_to_cpu(bg->bg_bits), > + le16_to_cpu(bg->bg_free_bits_count), num_bits); > + ret = -EROFS; > + goto out_rollback; > + } > + while (num_bits--) > + ocfs2_set_bit(bit_off++, bitmap); > + > ocfs2_journal_dirty(handle, di_bh); > + ocfs2_journal_dirty(handle, group_bh); > > out: > return ret; > + > +out_rollback: > + le16_add_cpu(&bg->bg_free_bits_count, num_bits); > + > + di->id1.bitmap1.i_used = cpu_to_le32(tmp_used - num_bits); > + le32_add_cpu(&cl->cl_recs[chain].c_free, num_bits); > + > + return ret; > } > > static int ocfs2_bg_discontig_fix_by_rec(struct ocfs2_suballoc_result *res, > @@ -1697,19 +1696,15 @@ static int ocfs2_search_one_group(struct > if (ac->ac_find_loc_only) > goto out_loc_only; > > - ret = ocfs2_alloc_dinode_update_counts(alloc_inode, handle, ac->ac_bh, > - res->sr_bits, > - le16_to_cpu(gd->bg_chain)); > + ret = ocfs2_alloc_dinode_update_bitmap(handle, > + alloc_inode, ac->ac_bh, gd, group_bh, > + le16_to_cpu(gd->bg_chain), > + res->sr_bit_offset, res->sr_bits); > if (ret < 0) { > mlog_errno(ret); > goto out; > } > > - ret = ocfs2_block_group_set_bits(handle, alloc_inode, gd, group_bh, > - res->sr_bit_offset, res->sr_bits); > - if (ret < 0) > - mlog_errno(ret); > - > out_loc_only: > *bits_left = le16_to_cpu(gd->bg_free_bits_count); > > @@ -1823,20 +1818,9 @@ static int ocfs2_search_chain(struct ocf > if (ac->ac_find_loc_only) > goto out_loc_only; > > - status = ocfs2_alloc_dinode_update_counts(alloc_inode, handle, > - ac->ac_bh, res->sr_bits, > - chain); > - if (status) { > - mlog_errno(status); > - goto bail; > - } > - > - status = ocfs2_block_group_set_bits(handle, > - alloc_inode, > - bg, > - group_bh, > - res->sr_bit_offset, > - res->sr_bits); > + status = ocfs2_alloc_dinode_update_bitmap(handle, > + alloc_inode, ac->ac_bh, bg, group_bh, > + chain, res->sr_bit_offset, res->sr_bits); > if (status < 0) { > mlog_errno(status); > goto bail; > @@ -2134,20 +2118,9 @@ int ocfs2_claim_new_inode_at_loc(handle_ > bg = (struct ocfs2_group_desc *) bg_bh->b_data; > chain = le16_to_cpu(bg->bg_chain); > > - ret = ocfs2_alloc_dinode_update_counts(ac->ac_inode, handle, > - ac->ac_bh, res->sr_bits, > - chain); > - if (ret) { > - mlog_errno(ret); > - goto out; > - } > - > - ret = ocfs2_block_group_set_bits(handle, > - ac->ac_inode, > - bg, > - bg_bh, > - res->sr_bit_offset, > - res->sr_bits); > + ret = ocfs2_alloc_dinode_update_bitmap(handle, > + ac->ac_inode, ac->ac_bh, bg, bg_bh, > + chain, res->sr_bit_offset, res->sr_bits); > if (ret < 0) { > mlog_errno(ret); > goto out; > diff -puN fs/ocfs2/suballoc.h~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously fs/ocfs2/suballoc.h > --- a/fs/ocfs2/suballoc.h~ocfs2-alloc_dinode-counts-and-group-bitmap-should-be-update-simultaneously > +++ a/fs/ocfs2/suballoc.h > @@ -85,19 +85,14 @@ int ocfs2_reserve_new_inode(struct ocfs2 > int ocfs2_reserve_clusters(struct ocfs2_super *osb, > u32 bits_wanted, > struct ocfs2_alloc_context **ac); > - > -int ocfs2_alloc_dinode_update_counts(struct inode *inode, > - handle_t *handle, > - struct buffer_head *di_bh, > - u32 num_bits, > - u16 chain); > -int ocfs2_block_group_set_bits(handle_t *handle, > - struct inode *alloc_inode, > - struct ocfs2_group_desc *bg, > - struct buffer_head *group_bh, > - unsigned int bit_off, > - unsigned int num_bits); > - > +int ocfs2_alloc_dinode_update_bitmap(handle_t *handle, > + struct inode *alloc_inode, > + struct buffer_head *di_bh, > + struct ocfs2_group_desc *bg, > + struct buffer_head *group_bh, > + u16 chain, > + u32 bit_off, > + u32 num_bits); > int ocfs2_claim_metadata(handle_t *handle, > struct ocfs2_alloc_context *ac, > u32 bits_wanted, > _ > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [PATCH] Ocfs2: Rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed 2014-03-31 13:07 [Ocfs2-devel] [PATCH] Ocfs2: Rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed Younger Liu 2014-03-31 21:02 ` Andrew Morton @ 2014-04-01 17:48 ` Mark Fasheh 1 sibling, 0 replies; 4+ messages in thread From: Mark Fasheh @ 2014-04-01 17:48 UTC (permalink / raw) To: ocfs2-devel On Mon, Mar 31, 2014 at 09:07:40PM +0800, Younger Liu wrote: > After updating alloc_dinode counts in ocfs2_alloc_dinode_update_counts(), > if ocfs2_alloc_dinode_update_bitmap() failed, there is a rare case that some > space may be lost. > So, we rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed. > > Signed-off-by: Younger Liu <younger.liucn@gmail.com> > Cc: Mark Fasheh <mfasheh@suse.de> Thanks for the latest version fo this patch Younger, it looks good. --Mark Reviewed-by: Mark Fasheh <mfasheh@suse.de> -- Mark Fasheh ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-04-01 17:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-31 13:07 [Ocfs2-devel] [PATCH] Ocfs2: Rollback alloc_dinode counts when ocfs2_block_group_set_bits() failed Younger Liu 2014-03-31 21:02 ` Andrew Morton 2014-04-01 0:37 ` Younger Liu 2014-04-01 17:48 ` Mark Fasheh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).