Heming Zhao
2022-Apr-13 08:29 UTC
[Ocfs2-devel] [PATCH v2 4/5] ocfs2: ocfs2_mount_volume does cleanup job before return error
After this patch, when error, ocfs2_fill_super doesn't take care to
release resources which are allocated in ocfs2_mount_volume.
Signed-off-by: Heming Zhao <heming.zhao at suse.com>
---
fs/ocfs2/super.c | 42 +++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 4302c3e9598c..5e860d7162d7 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1803,11 +1803,10 @@ static int ocfs2_get_sector(struct super_block *sb,
static int ocfs2_mount_volume(struct super_block *sb)
{
int status = 0;
- int unlock_super = 0;
struct ocfs2_super *osb = OCFS2_SB(sb);
if (ocfs2_is_hard_readonly(osb))
- goto leave;
+ goto out;
mutex_init(&osb->obs_trim_fs_mutex);
@@ -1817,44 +1816,54 @@ static int ocfs2_mount_volume(struct super_block *sb)
if (status == -EBADR && ocfs2_userspace_stack(osb))
mlog(ML_ERROR, "couldn't mount because cluster name on"
" disk does not match the running cluster name.\n");
- goto leave;
+ goto out;
}
status = ocfs2_super_lock(osb, 1);
if (status < 0) {
mlog_errno(status);
- goto leave;
+ goto out_dlm;
}
- unlock_super = 1;
/* This will load up the node map and add ourselves to it. */
status = ocfs2_find_slot(osb);
if (status < 0) {
mlog_errno(status);
- goto leave;
+ goto out_super_lock;
}
/* load all node-local system inodes */
status = ocfs2_init_local_system_inodes(osb);
if (status < 0) {
mlog_errno(status);
- goto leave;
+ goto out_super_lock;
}
status = ocfs2_check_volume(osb);
if (status < 0) {
mlog_errno(status);
- goto leave;
+ goto out_system_inodes;
}
status = ocfs2_truncate_log_init(osb);
- if (status < 0)
+ if (status < 0) {
mlog_errno(status);
+ goto out_system_inodes;
+ }
-leave:
- if (unlock_super)
- ocfs2_super_unlock(osb, 1);
+ ocfs2_super_unlock(osb, 1);
+ return 0;
+out_system_inodes:
+ if (osb->local_alloc_state == OCFS2_LA_ENABLED)
+ ocfs2_shutdown_local_alloc(osb);
+ ocfs2_release_system_inodes(osb);
+ ocfs2_journal_shutdown(osb);
+out_super_lock:
+ ocfs2_super_unlock(osb, 1);
+out_dlm:
+ ocfs2_dlm_shutdown(osb, 0);
+out:
return status;
}
@@ -2393,14 +2402,17 @@ static int ocfs2_verify_volume(struct ocfs2_dinode *di,
return status;
}
+/*
+ * If this function returns failure, caller responds to release
+ * here alloced resources.
+ */
static int ocfs2_check_volume(struct ocfs2_super *osb)
{
int status;
int dirty;
int local;
- struct ocfs2_dinode *local_alloc = NULL; /* only used if we
- * recover
- * ourselves. */
+ /* only used if we recover ourselves. */
+ struct ocfs2_dinode *local_alloc = NULL;
/* Init our journal object. */
status = ocfs2_journal_init(osb, &dirty);
--
2.35.1
Joseph Qi
2022-Apr-13 11:25 UTC
[Ocfs2-devel] [PATCH v2 4/5] ocfs2: ocfs2_mount_volume does cleanup job before return error
On 4/13/22 4:29 PM, Heming Zhao wrote:> After this patch, when error, ocfs2_fill_super doesn't take care to > release resources which are allocated in ocfs2_mount_volume. > > Signed-off-by: Heming Zhao <heming.zhao at suse.com> > --- > fs/ocfs2/super.c | 42 +++++++++++++++++++++++++++--------------- > 1 file changed, 27 insertions(+), 15 deletions(-) > > diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c > index 4302c3e9598c..5e860d7162d7 100644 > --- a/fs/ocfs2/super.c > +++ b/fs/ocfs2/super.c > @@ -1803,11 +1803,10 @@ static int ocfs2_get_sector(struct super_block *sb, > static int ocfs2_mount_volume(struct super_block *sb) > { > int status = 0; > - int unlock_super = 0; > struct ocfs2_super *osb = OCFS2_SB(sb); > > if (ocfs2_is_hard_readonly(osb)) > - goto leave; > + goto out; > > mutex_init(&osb->obs_trim_fs_mutex); > > @@ -1817,44 +1816,54 @@ static int ocfs2_mount_volume(struct super_block *sb) > if (status == -EBADR && ocfs2_userspace_stack(osb)) > mlog(ML_ERROR, "couldn't mount because cluster name on" > " disk does not match the running cluster name.\n"); > - goto leave; > + goto out; > } > > status = ocfs2_super_lock(osb, 1); > if (status < 0) { > mlog_errno(status); > - goto leave; > + goto out_dlm; > } > - unlock_super = 1; > > /* This will load up the node map and add ourselves to it. */ > status = ocfs2_find_slot(osb); > if (status < 0) { > mlog_errno(status); > - goto leave; > + goto out_super_lock; > } > > /* load all node-local system inodes */ > status = ocfs2_init_local_system_inodes(osb); > if (status < 0) { > mlog_errno(status); > - goto leave; > + goto out_super_lock; > } > > status = ocfs2_check_volume(osb); > if (status < 0) { > mlog_errno(status); > - goto leave; > + goto out_system_inodes; > } > > status = ocfs2_truncate_log_init(osb); > - if (status < 0) > + if (status < 0) { > mlog_errno(status); > + goto out_system_inodes; > + } > > -leave: > - if (unlock_super) > - ocfs2_super_unlock(osb, 1); > + ocfs2_super_unlock(osb, 1); > + return 0; > > +out_system_inodes: > + if (osb->local_alloc_state == OCFS2_LA_ENABLED) > + ocfs2_shutdown_local_alloc(osb); > + ocfs2_release_system_inodes(osb); > + ocfs2_journal_shutdown(osb); > +out_super_lock: > + ocfs2_super_unlock(osb, 1); > +out_dlm: > + ocfs2_dlm_shutdown(osb, 0); > +out: > return status; > } > > @@ -2393,14 +2402,17 @@ static int ocfs2_verify_volume(struct ocfs2_dinode *di, > return status; > } > > +/* > + * If this function returns failure, caller responds to release > + * here alloced resources. > + */ > static int ocfs2_check_volume(struct ocfs2_super *osb) > { > int status; > int dirty; > int local; > - struct ocfs2_dinode *local_alloc = NULL; /* only used if we > - * recover > - * ourselves. */ > + /* only used if we recover ourselves. */Malformed. Actually I don't think we have to touch this function. Thanks, Joseph> + struct ocfs2_dinode *local_alloc = NULL; > > /* Init our journal object. */ > status = ocfs2_journal_init(osb, &dirty);