On Fri, May 5, 2017 at 9:34 AM, Rock Lee <rockdotlee@xxxxxxxxx> wrote: > Hi, Amir: > >> This is not a leak. dir cache is refcounted and freed on >> ovl_dir_{release|reset}() -> ovl_cache_put() > > Here is the snippet: > > if (cache && ovl_dentry_version_get(dentry) == cache->version) { > cache->refcount++; > return cache; > } > > ovl_set_dir_cache(dentry, NULL); > > I am a little confused here. From the logic of the code, there must > have a circumstance --- cache is not NULL and > ovl_dentry_version_get(dentry) != cache->version. If > ovl_set_dir_cache(dentry, NULL), we can't find the old dir cache form > dentry and struct file(later, file->private_data->cache will point at > the same dir cache with dentry). How could we free it in > ovl_dir_{release|reset} ? > It has a reference from another struct file, not this one and cache will be freed when the last file that references it is closed: static int ovl_dir_release(struct inode *inode, struct file *file) { struct ovl_dir_file *od = file->private_data; if (od->cache) { inode_lock(inode); ovl_cache_put(od, file->f_path.dentry); inode_unlock(inode); } > BTW, could you please an example when ovl_dentry_version_get(dentry) > != cache->version in ovl_cache_get ? > In theory (didn't try this code): d1 = opendir("foo"); readdir(d1); /* populate d1's od->cache and read first entry */ pos = telldir(d1); creat("foo/bar", 0600); /* ovl_dentry_version_inc() */ d2 = opendir("foo"); seekdir(d2, pos); readdir(d2); /* populate d2's od->cache and read second entry */ d2's od->cache is NULL, ctx->pos = 1, and ovl_get_cache() will find that ovl_dir_cache() is out dated and will create a new cache, which should contain the new file "bar". Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html