[CRIU] [PATCH] mount: mnt_entry_alloc -- Don't forget to init @mnt_slave

Cyrill Gorcunov gorcunov at gmail.com
Tue Sep 1 03:12:07 PDT 2015


On Tue, Sep 01, 2015 at 12:32:31PM +0300, Pavel Emelyanov wrote:
> > 
> > Because it's a separate routine called "mnt_entry_alloc",
> > it has no clue how would we use any of this list entries.
> > So I think it's better to init this entry, frankly its
> > just a few cpu ops.
> 
> Kernel doesn't initialize list-entries without need either :) Only
> heads. So I'd just add a comment somewhere saying this.

static struct mount *alloc_vfsmnt(const char *name)
{
	struct mount *mnt = kmem_cache_zalloc(mnt_cache, GFP_KERNEL);
	...
		INIT_LIST_HEAD(&mnt->mnt_share);
		INIT_LIST_HEAD(&mnt->mnt_slave_list);
		INIT_LIST_HEAD(&mnt->mnt_slave);
	...
}

Really, such kind of things are simply waiting for trigger the bugs.
Instead of adding comment do a right thing and simply init the entry.
Yes, we have a couple of places in criu where we don't init list
entries but the difference is that we allocate and add them immediately,
not somewhere "later" in code. For example

open_remap_ghost
	gf = shmalloc(sizeof(*gf));
	...
	list_add_tail(&gf->list, &ghost_files);

that's fine, because we allocate and collect in one place.

Anyway, this aspect doesn't worth to waste time on it so if
you both don't like it I can live with that, drop it.

	Cyrill


More information about the CRIU mailing list