* [Ocfs2-devel] [PATCH] ocfs2: free meta_ac and data_ac when ocfs2_start_trans fails in ocfs2_xattr_set() @ 2013-08-01 9:02 Younger Liu 2013-08-06 22:44 ` Jeff Liu 0 siblings, 1 reply; 4+ messages in thread From: Younger Liu @ 2013-08-01 9:02 UTC (permalink / raw) To: ocfs2-devel In ocfs2_xattr_set(), if ocfs2_start_trans failed, meta_ac and data_ac should be free. Otherwise, It would lead to memory leak. Signed-off-by: Younger Liu <younger.liu@huawei.com> Cc: Joseph Qi <joseph.qi@huawei.com> --- fs/ocfs2/xattr.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 317ef0a..1c2a892 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -3505,7 +3505,7 @@ int ocfs2_xattr_set(struct inode *inode, int ret, credits, ref_meta = 0, ref_credits = 0; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct inode *tl_inode = osb->osb_tl_inode; - struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, }; + struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, NULL, }; struct ocfs2_refcount_tree *ref_tree = NULL; struct ocfs2_xattr_info xi = { @@ -3609,13 +3609,14 @@ int ocfs2_xattr_set(struct inode *inode, if (IS_ERR(ctxt.handle)) { ret = PTR_ERR(ctxt.handle); mlog_errno(ret); - goto cleanup; + goto out_free_ac; } ret = __ocfs2_xattr_set_handle(inode, di, &xi, &xis, &xbs, &ctxt); ocfs2_commit_trans(osb, ctxt.handle); +out_free_ac: if (ctxt.data_ac) ocfs2_free_alloc_context(ctxt.data_ac); if (ctxt.meta_ac) -- 1.7.9.7 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: free meta_ac and data_ac when ocfs2_start_trans fails in ocfs2_xattr_set() 2013-08-01 9:02 [Ocfs2-devel] [PATCH] ocfs2: free meta_ac and data_ac when ocfs2_start_trans fails in ocfs2_xattr_set() Younger Liu @ 2013-08-06 22:44 ` Jeff Liu 2013-08-07 2:03 ` Younger Liu 0 siblings, 1 reply; 4+ messages in thread From: Jeff Liu @ 2013-08-06 22:44 UTC (permalink / raw) To: ocfs2-devel On 08/01/2013 05:02 PM, Younger Liu wrote: > In ocfs2_xattr_set(), if ocfs2_start_trans failed, > meta_ac and data_ac should be free. > Otherwise, It would lead to memory leak. > > Signed-off-by: Younger Liu <younger.liu@huawei.com> > Cc: Joseph Qi <joseph.qi@huawei.com> > --- > fs/ocfs2/xattr.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c > index 317ef0a..1c2a892 100644 > --- a/fs/ocfs2/xattr.c > +++ b/fs/ocfs2/xattr.c > @@ -3505,7 +3505,7 @@ int ocfs2_xattr_set(struct inode *inode, > int ret, credits, ref_meta = 0, ref_credits = 0; > struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); > struct inode *tl_inode = osb->osb_tl_inode; > - struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, }; > + struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, NULL, }; > struct ocfs2_refcount_tree *ref_tree = NULL; > > struct ocfs2_xattr_info xi = { > @@ -3609,13 +3609,14 @@ int ocfs2_xattr_set(struct inode *inode, > if (IS_ERR(ctxt.handle)) { > ret = PTR_ERR(ctxt.handle); > mlog_errno(ret); > - goto cleanup; > + goto out_free_ac; > } > > ret = __ocfs2_xattr_set_handle(inode, di, &xi, &xis, &xbs, &ctxt); > > ocfs2_commit_trans(osb, ctxt.handle); > > +out_free_ac: > if (ctxt.data_ac) > ocfs2_free_alloc_context(ctxt.data_ac); > if (ctxt.meta_ac) Yes, we should free the allocated contex in this case. However, we don't performs the log truncation as well as the delayed allocation business as per the old logic. Srinivas, How do you think about this fix? Thanks, -Jeff ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: free meta_ac and data_ac when ocfs2_start_trans fails in ocfs2_xattr_set() 2013-08-06 22:44 ` Jeff Liu @ 2013-08-07 2:03 ` Younger Liu 2013-08-07 7:21 ` Jeff Liu 0 siblings, 1 reply; 4+ messages in thread From: Younger Liu @ 2013-08-07 2:03 UTC (permalink / raw) To: ocfs2-devel On 2013/8/7 6:44, Jeff Liu wrote: > On 08/01/2013 05:02 PM, Younger Liu wrote: > >> In ocfs2_xattr_set(), if ocfs2_start_trans failed, >> meta_ac and data_ac should be free. >> Otherwise, It would lead to memory leak. >> >> Signed-off-by: Younger Liu <younger.liu@huawei.com> >> Cc: Joseph Qi <joseph.qi@huawei.com> >> --- >> fs/ocfs2/xattr.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c >> index 317ef0a..1c2a892 100644 >> --- a/fs/ocfs2/xattr.c >> +++ b/fs/ocfs2/xattr.c >> @@ -3505,7 +3505,7 @@ int ocfs2_xattr_set(struct inode *inode, >> int ret, credits, ref_meta = 0, ref_credits = 0; >> struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); >> struct inode *tl_inode = osb->osb_tl_inode; >> - struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, }; >> + struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, NULL, }; >> struct ocfs2_refcount_tree *ref_tree = NULL; >> >> struct ocfs2_xattr_info xi = { >> @@ -3609,13 +3609,14 @@ int ocfs2_xattr_set(struct inode *inode, >> if (IS_ERR(ctxt.handle)) { >> ret = PTR_ERR(ctxt.handle); >> mlog_errno(ret); >> - goto cleanup; >> + goto out_free_ac; >> } >> >> ret = __ocfs2_xattr_set_handle(inode, di, &xi, &xis, &xbs, &ctxt); >> >> ocfs2_commit_trans(osb, ctxt.handle); >> >> +out_free_ac: >> if (ctxt.data_ac) >> ocfs2_free_alloc_context(ctxt.data_ac); >> if (ctxt.meta_ac) > > Yes, we should free the allocated contex in this case. > However, we don't performs the log truncation as well as the delayed allocation > business as per the old logic. > > Srinivas, > > How do you think about this fix? > > Thanks, > -Jeff > In ocfs2_init_xattr_set_ctxt(), the allocated contex is initialized, (&ctxt.dealloc)->c_global_allocator and (&ctxt.dealloc)-> c_first_suballocator are set to NULL. After applying the patch, if ocfs2_start_trans fails, we would free meta_ac/data_ac, but not perform truncate_log_flush/ocfs2_run_deallocs. So, it would take no side effect to the function. Thanks. --Younger ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: free meta_ac and data_ac when ocfs2_start_trans fails in ocfs2_xattr_set() 2013-08-07 2:03 ` Younger Liu @ 2013-08-07 7:21 ` Jeff Liu 0 siblings, 0 replies; 4+ messages in thread From: Jeff Liu @ 2013-08-07 7:21 UTC (permalink / raw) To: ocfs2-devel On 08/07/2013 10:03 AM, Younger Liu wrote: > On 2013/8/7 6:44, Jeff Liu wrote: >> On 08/01/2013 05:02 PM, Younger Liu wrote: >> >>> In ocfs2_xattr_set(), if ocfs2_start_trans failed, >>> meta_ac and data_ac should be free. >>> Otherwise, It would lead to memory leak. >>> >>> Signed-off-by: Younger Liu <younger.liu@huawei.com> >>> Cc: Joseph Qi <joseph.qi@huawei.com> >>> --- >>> fs/ocfs2/xattr.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c >>> index 317ef0a..1c2a892 100644 >>> --- a/fs/ocfs2/xattr.c >>> +++ b/fs/ocfs2/xattr.c >>> @@ -3505,7 +3505,7 @@ int ocfs2_xattr_set(struct inode *inode, >>> int ret, credits, ref_meta = 0, ref_credits = 0; >>> struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); >>> struct inode *tl_inode = osb->osb_tl_inode; >>> - struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, }; >>> + struct ocfs2_xattr_set_ctxt ctxt = { NULL, NULL, NULL, }; >>> struct ocfs2_refcount_tree *ref_tree = NULL; >>> >>> struct ocfs2_xattr_info xi = { >>> @@ -3609,13 +3609,14 @@ int ocfs2_xattr_set(struct inode *inode, >>> if (IS_ERR(ctxt.handle)) { >>> ret = PTR_ERR(ctxt.handle); >>> mlog_errno(ret); >>> - goto cleanup; >>> + goto out_free_ac; >>> } >>> >>> ret = __ocfs2_xattr_set_handle(inode, di, &xi, &xis, &xbs, &ctxt); >>> >>> ocfs2_commit_trans(osb, ctxt.handle); >>> >>> +out_free_ac: >>> if (ctxt.data_ac) >>> ocfs2_free_alloc_context(ctxt.data_ac); >>> if (ctxt.meta_ac) >> >> Yes, we should free the allocated contex in this case. >> However, we don't performs the log truncation as well as the delayed allocation s/delayed allocation/delalloc/. Sorry, I habitually typed above wrong. >> business as per the old logic. >> >> Srinivas, >> >> How do you think about this fix? >> >> Thanks, >> -Jeff >> > In ocfs2_init_xattr_set_ctxt(), the allocated contex is initialized, > (&ctxt.dealloc)->c_global_allocator and (&ctxt.dealloc)-> > c_first_suballocator are set to NULL. > > After applying the patch, if ocfs2_start_trans fails, we would free > meta_ac/data_ac, but not perform truncate_log_flush/ocfs2_run_deallocs. > So, it would take no side effect to the function. So this fix looks good, thanks. Reviewed-by: Jie Liu <jeff.liu@oracle.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-07 7:21 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-01 9:02 [Ocfs2-devel] [PATCH] ocfs2: free meta_ac and data_ac when ocfs2_start_trans fails in ocfs2_xattr_set() Younger Liu 2013-08-06 22:44 ` Jeff Liu 2013-08-07 2:03 ` Younger Liu 2013-08-07 7:21 ` Jeff Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.