- bm_data = kzalloc(sizeof(struct binfmt_misc), GFP_KERNEL);
- if (!bm_data)
- return -ENOMEM;
+ INIT_LIST_HEAD(&bm_data->entries);
+ rwlock_init(&bm_data->entries_lock);
- INIT_LIST_HEAD(&bm_data->entries);
- rwlock_init(&bm_data->entries_lock);
+ ve->binfmt_misc = bm_data;
Isn't it better to move ve->binfmt_misc assignment to the
end of function where we know that all operations was successful?
Since ve->binfmt_misc is global, not per-mount, the logic is simpler if creation of bm_data is made
independent from mount success/failure. I.e. regardless of success of mount operation, bm_data is
created at the first mount and saved in ve->binfmt_misc. Then, it will be cleaned at ve destruction time.
+ /* this will be cleared by ve_binfmt_fini() */
+ }
err = simple_fill_super(sb, BINFMTFS_MAGIC, bm_files);
- if (err) {
- kfree(bm_data);
If we have ve->binfmt_misc assignment in the upper part of code, then
we need to do ve->binfmt_misc = NULL here.
This will mishandle case when ve->binfmt_misc was initialized at previous mount.
+ if (err)
return err;
- }
sb->s_op = &s_ops;
-
- ve->binfmt_misc = bm_data;
see above
bm_data->enabled = 1;
return 0;
@@ -971,6 +958,8 @@ static void ve_binfmt_fini(void *data)
while (!list_empty(&bm_data->entries))
kill_node(bm_data, list_first_entry(
&bm_data->entries, Node, list));
+
+ kfree(bm_data);
We have kfree in ve_destroy (kernel/ve/ve.c) already.
Indeed.
But, why splitting destruction into two parts?
Why not doing both at the same location.
Nikita
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
