On Sun, May 7, 2017 at 7:34 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> 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".
Thanks for your explaination, I leaned a lot. I have tested your code,
and it met your expectation ;-).
One more question, only ctx->pos == 0 in ovl_iterate would cause
ovl_dir_reset and rebuild dir cache which means in some scenarios, dir
cache may be not up to date. Consider this code:
DIR *dirp = opendir("foo");
readdir(dirp); /* make sure ctx->pos != 0 in the
blow readir() */
creat("foo/bar" ,0600); /* ovl_dentry_version_inc() */
while ((entry = readdir(dirp)) != NULL) {
printf("name %s\n", entry->d_name);
}
In this case, "foo/bar" will not be shown in the output because
readdir will not rebuild dir cache. Could we just ovl_dir_reset()
everytime regardless of ctx->pos in ovl_iterate() ? Once files
created/removed/renamed in a directory, ovl_iterate will rebuild dir
cache and keep the dir cache up to date?
--
Cheers,
Rock
--
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
