| Message ID | 20240821024301.1058918-5-wozizhi@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | netfs/cachefiles: Some bugfixes | expand |
Zizhi Wo <wozizhi@huawei.com> wrote: > In the current on-demand loading scenario, when umount is called, the > cachefiles_commit_tmpfile() is invoked. When checking the inode > corresponding to object->file is inconsistent with the dentry, > cachefiles_unlink() is called to perform cleanup to prevent invalid data > from occupying space. > > The above operation does not apply to the first mount, because the cache > dentry generated by the first mount must be negative. Moreover, there is no > need to clear it during the first umount because this part of the data may > be reusable in the future. But the problem is that, the clean operation can > currently only be called through cachefiles_withdraw_cookie(), in other > words the redundant data does not cleaned until the second umount. This > means that during the second mount, the old cache data generated from the > first mount still occupies space. So if the user does not manually clean up > the previous cache before the next mount, it may return insufficient space > during the second mount phase. > > This patch adds an additional cleanup process in the cachefiles_open_file() > function. When the auxdata check fails, the remaining old cache data is no > longer needed, the file and dentry corresponding to the object are also > put. As there is no need to clear it until umount, we can directly clear it > during the mount process. > > Signed-off-by: Zizhi Wo <wozizhi@huawei.com> Okay, I think this is reasonable as it's done from a worker thread. I wonder if instead, though, cachefiles_create_file() should be called and then linked over the top: https://lore.kernel.org/all/cover.1580251857.git.osandov@fb.com/ though AT_LINK_REPLACE seemed to get stuck. Note that we can't just truncate the file to nothing instead because I/O might be in progress on it. David
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index f53977169db4..70b0b3477085 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -542,7 +542,7 @@ static bool cachefiles_create_file(struct cachefiles_object *object) * stale. */ static bool cachefiles_open_file(struct cachefiles_object *object, - struct dentry *dentry) + struct dentry *dir, struct dentry *dentry) { struct cachefiles_cache *cache = object->volume->cache; struct file *file; @@ -601,10 +601,18 @@ static bool cachefiles_open_file(struct cachefiles_object *object, check_failed: fscache_cookie_lookup_negative(object->cookie); cachefiles_unmark_inode_in_use(object, file); - fput(file); - dput(dentry); - if (ret == -ESTALE) + __fput_sync(file); + if (ret == -ESTALE) { + /* When the auxdata check fails, the remaining old cache data + * is no longer needed, and we will clear it here first. + */ + inode_lock_nested(d_inode(dir), I_MUTEX_PARENT); + cachefiles_unlink(cache, object, dir, dentry, FSCACHE_OBJECT_IS_STALE); + inode_unlock(d_inode(dir)); + dput(dentry); return cachefiles_create_file(object); + } + dput(dentry); return false; error_fput: @@ -654,7 +662,7 @@ bool cachefiles_look_up_object(struct cachefiles_object *object) goto new_file; } - if (!cachefiles_open_file(object, dentry)) + if (!cachefiles_open_file(object, fan, dentry)) return false; _leave(" = t [%lu]", file_inode(object->file)->i_ino);
In the current on-demand loading scenario, when umount is called, the cachefiles_commit_tmpfile() is invoked. When checking the inode corresponding to object->file is inconsistent with the dentry, cachefiles_unlink() is called to perform cleanup to prevent invalid data from occupying space. The above operation does not apply to the first mount, because the cache dentry generated by the first mount must be negative. Moreover, there is no need to clear it during the first umount because this part of the data may be reusable in the future. But the problem is that, the clean operation can currently only be called through cachefiles_withdraw_cookie(), in other words the redundant data does not cleaned until the second umount. This means that during the second mount, the old cache data generated from the first mount still occupies space. So if the user does not manually clean up the previous cache before the next mount, it may return insufficient space during the second mount phase. This patch adds an additional cleanup process in the cachefiles_open_file() function. When the auxdata check fails, the remaining old cache data is no longer needed, the file and dentry corresponding to the object are also put. As there is no need to clear it until umount, we can directly clear it during the mount process. Signed-off-by: Zizhi Wo <wozizhi@huawei.com> --- fs/cachefiles/namei.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)