Messages in this thread Patches in this message | | | Date | Fri, 24 Jan 2014 13:20:48 -0500 | From | Dave Jones <> | Subject | Re: slab: clean up kmem_cache_create_memcg() error handling |
| |
On Fri, Jan 24, 2014 at 03:33:41AM +0000, Linux Kernel wrote: > Gitweb: http://git.kernel.org/linus/;a=commit;h=3965fc3652244651006ebb31c8c45318ce84818f > Commit: 3965fc3652244651006ebb31c8c45318ce84818f > Parent: 309381feaee564281c3d9e90fbca8963bb7428ad > Author: Vladimir Davydov <vdavydov@parallels.com> > AuthorDate: Thu Jan 23 15:52:55 2014 -0800 > Committer: Linus Torvalds <torvalds@linux-foundation.org> > CommitDate: Thu Jan 23 16:36:50 2014 -0800 > > slab: clean up kmem_cache_create_memcg() error handling > > Currently kmem_cache_create_memcg() backoffs on failure inside > conditionals, without using gotos. This results in the rollback code > duplication, which makes the function look cumbersome even though on > error we should only free the allocated cache. Since in the next patch > I am going to add yet another rollback function call on error path > there, let's employ labels instead of conditionals for undoing any > changes on failure to keep things clean.
...
> +out_unlock: > mutex_unlock(&slab_mutex); > put_online_cpus(); > > if (err) { > - > if (flags & SLAB_PANIC) > panic("kmem_cache_create: Failed to create slab '%s'. Error %d\n", > name, err); > @@ -236,11 +230,14 @@ out_locked: > name, err); > dump_stack(); > } > - > return NULL; > } > - > return s; > + > +out_free_cache: > + kfree(s->name); > + kmem_cache_free(kmem_cache, s); > + goto out_unlock; > } This is now returning a freed pointer as 's' if an error occurs. Perhaps the patch below ?
Dave
diff --git a/mm/slab_common.c b/mm/slab_common.c index 8e40321da091..2c62294cee23 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -257,6 +257,7 @@ out_free_cache: memcg_free_cache_params(s); kfree(s->name); kmem_cache_free(kmem_cache, s); + s = NULL; goto out_unlock; }
| |