[PATCH 45/67] cachefiles: Simplify the file lookup/creation/check code

David Howells
2021-10-18T15:02:53Z

Simplify the code that does file lookup, creation and checking.

Signed-off-by: David Howells <dhowells@redhat.com>
---

fs/cachefiles/interface.c | 4 -
fs/cachefiles/internal.h | 11 +
fs/cachefiles/io.c | 2
fs/cachefiles/namei.c | 276 ++++++++++++++++++++-----------------
fs/cachefiles/xattr.c | 6 -
include/trace/events/cachefiles.h | 2
6 files changed, 160 insertions(+), 141 deletions(-)

diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c
index 96a703d5f62c..38ae34b7aaf4 100644
--- a/fs/cachefiles/interface.c
+++ b/fs/cachefiles/interface.c
@@ -79,7 +79,7 @@ static bool cachefiles_lookup_cookie(struct fscache_cookie *cookie)

/* look up the key, creating any missing bits */
cachefiles_begin_secure(cache, &saved_cred);
- success = cachefiles_walk_to_object(object);
+ success = cachefiles_look_up_object(object);
cachefiles_end_secure(cache, saved_cred);

if (!success)
@@ -222,7 +222,7 @@ static void cachefiles_clean_up_object(struct cachefiles_object *object,
cachefiles_commit_object(object, cache);
}

- cachefiles_unmark_inode_in_use(object);
+ cachefiles_unmark_inode_in_use(object, object->file);
if (object->file) {
fput(object->file);
object->file = NULL;
diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index 6cc22c85c8f2..a2d2ed2f19eb 100644
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -58,8 +58,7 @@ struct cachefiles_object {
u8 d_name_len; /* Length of filename */
u8 key_hash; /* Hash of object key */
unsigned long flags;
-#define CACHEFILES_OBJECT_IS_NEW 0 /* Set if object is new */
-#define CACHEFILES_OBJECT_USING_TMPFILE 1 /* Have an unlinked tmpfile */
+#define CACHEFILES_OBJECT_USING_TMPFILE 0 /* Have an unlinked tmpfile */
};

extern struct kmem_cache *cachefiles_object_jar;
@@ -171,7 +170,8 @@ extern bool cachefiles_cook_key(struct cachefiles_object *object);
/*
* namei.c
*/
-extern void cachefiles_unmark_inode_in_use(struct cachefiles_object *object);
+extern void cachefiles_unmark_inode_in_use(struct cachefiles_object *object,
+ struct file *file);
extern int cachefiles_bury_object(struct cachefiles_cache *cache,
struct cachefiles_object *object,
struct dentry *dir,
@@ -179,7 +179,7 @@ extern int cachefiles_bury_object(struct cachefiles_cache *cache,
enum fscache_why_object_killed why);
extern int cachefiles_delete_object(struct cachefiles_object *object,
enum fscache_why_object_killed why);
-extern bool cachefiles_walk_to_object(struct cachefiles_object *object);
+extern bool cachefiles_look_up_object(struct cachefiles_object *object);
extern struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
struct dentry *dir,
const char *name);
@@ -224,7 +224,8 @@ void cachefiles_withdraw_volume(struct cachefiles_volume *volume);
* xattr.c
*/
extern int cachefiles_set_object_xattr(struct cachefiles_object *object);
-extern int cachefiles_check_auxdata(struct cachefiles_object *object);
+extern int cachefiles_check_auxdata(struct cachefiles_object *object,
+ struct file *file);
extern int cachefiles_remove_object_xattr(struct cachefiles_cache *cache,
struct dentry *dentry);

diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
index 67ea9f44931f..9b3b55a94e66 100644
--- a/fs/cachefiles/io.c
+++ b/fs/cachefiles/io.c
@@ -525,7 +525,7 @@ bool cachefiles_begin_operation(struct netfs_cache_resources *cres,

if (!cachefiles_cres_file(cres)) {
cres->ops = &cachefiles_netfs_cache_ops;
- if (object) {
+ if (object->file) {
spin_lock(&object->lock);
if (!cres->cache_priv2 && object->file)
cres->cache_priv2 = get_file(object->file);
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 0edf1276768b..989df918570b 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -21,9 +21,10 @@
/*
* Mark the backing file as being a cache file if it's not already in use so.
*/
-static bool cachefiles_mark_inode_in_use(struct cachefiles_object *object)
+static bool cachefiles_mark_inode_in_use(struct cachefiles_object *object,
+ struct dentry *dentry)
{
- struct inode *inode = file_inode(object->file);
+ struct inode *inode = d_backing_inode(dentry);
bool can_use = false;

_enter(",%x", object->debug_id);
@@ -45,9 +46,10 @@ static bool cachefiles_mark_inode_in_use(struct cachefiles_object *object)
/*
* Unmark a backing inode.
*/
-void cachefiles_unmark_inode_in_use(struct cachefiles_object *object)
+void cachefiles_unmark_inode_in_use(struct cachefiles_object *object,
+ struct file *file)
{
- struct inode *inode = file_inode(object->file);
+ struct inode *inode = file_inode(file);

if (!inode)
return;
@@ -61,10 +63,11 @@ void cachefiles_unmark_inode_in_use(struct cachefiles_object *object)
/*
* Mark an object as being inactive.
*/
-static void cachefiles_mark_object_inactive(struct cachefiles_object *object)
+static void cachefiles_mark_object_inactive(struct cachefiles_object *object,
+ struct file *file)
{
struct cachefiles_cache *cache = object->volume->cache;
- blkcnt_t i_blocks = file_inode(object->file)->i_blocks;
+ blkcnt_t i_blocks = file_inode(file)->i_blocks;

/* This object can now be culled, so we need to let the daemon know
* that there is something it can remove if it needs to.
@@ -232,191 +235,180 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
return 0;
}

+static int cachefiles_unlink(struct cachefiles_object *object,
+ struct dentry *fan, struct dentry *dentry,
+ enum fscache_why_object_killed why)
+{
+ struct path path = {
+ .mnt = object->volume->cache->mnt,
+ .dentry = fan,
+ };
+ int ret;
+
+ trace_cachefiles_unlink(object, dentry, why);
+ ret = security_path_unlink(&path, dentry);
+ if (ret == 0)
+ ret = vfs_unlink(&init_user_ns, d_backing_inode(fan), dentry, NULL);
+ return ret;
+}
+
/*
- * delete an object representation from the cache
+ * Delete a cache file.
*/
int cachefiles_delete_object(struct cachefiles_object *object,
enum fscache_why_object_killed why)
{
struct cachefiles_volume *volume = object->volume;
+ struct dentry *dentry = object->file->f_path.dentry;
struct dentry *fan = volume->fanout[(u8)object->key_hash];
+ int ret;

_enter(",OBJ%x{%pD}", object->debug_id, object->file);

+ /* Stop the dentry being negated if it's only pinned by a file struct. */
+ dget(dentry);
+
inode_lock_nested(d_backing_inode(fan), I_MUTEX_PARENT);
- return cachefiles_bury_object(volume->cache, object, fan,
- object->file->f_path.dentry, why);
+ ret = cachefiles_unlink(object, fan, dentry, why);
+ inode_unlock(d_backing_inode(fan));
+ dput(dentry);
+
+ if (ret < 0 && ret != -ENOENT)
+ cachefiles_io_error(volume->cache, "Unlink failed");
+ return ret;
}

/*
- * Check the attributes on a file we've just opened and delete it if it's out
- * of date.
+ * Create a new file.
*/
-static int cachefiles_check_open_object(struct cachefiles_object *object,
- struct dentry *fan)
+static bool cachefiles_create_file(struct cachefiles_object *object)
{
+ struct file *file;
int ret;

- if (!cachefiles_mark_inode_in_use(object))
- return -EBUSY;
-
- _enter("%pD", object->file);
-
- ret = cachefiles_check_auxdata(object);
- if (ret == -ESTALE)
- goto stale;
+ ret = cachefiles_has_space(object->volume->cache, 1, 0);
if (ret < 0)
- goto error_unmark;
+ return false;

- /* Always update the atime on an object we've just looked up (this is
- * used to keep track of culling, and atimes are only updated by read,
- * write and readdir but not lookup or open).
- */
- touch_atime(&object->file->f_path);
- return 0;
-
-stale:
- set_bit(CACHEFILES_OBJECT_IS_NEW, &object->flags);
- fscache_cookie_lookup_negative(object->cookie);
- cachefiles_unmark_inode_in_use(object);
- inode_lock_nested(d_inode(fan), I_MUTEX_PARENT);
- ret = cachefiles_bury_object(object->volume->cache, object, fan,
- object->file->f_path.dentry,
- FSCACHE_OBJECT_IS_STALE);
- if (ret < 0)
- return ret;
- cachefiles_mark_object_inactive(object);
- _debug("redo lookup");
- return -ESTALE;
+ file = cachefiles_create_tmpfile(object);
+ if (IS_ERR(file))
+ return false;

-error_unmark:
- cachefiles_unmark_inode_in_use(object);
- return ret;
+ set_bit(FSCACHE_COOKIE_NEEDS_UPDATE, &object->cookie->flags);
+ set_bit(CACHEFILES_OBJECT_USING_TMPFILE, &object->flags);
+ _debug("create -> %pD{ino=%lu}", file, file_inode(file)->i_ino);
+ object->file = file;
+ return true;
}

/*
- * Look up a file, creating it if necessary.
+ * Open an existing file, checking its attributes and replacing it if it is
+ * stale.
*/
-static int cachefiles_open_file(struct cachefiles_object *object,
- struct dentry *fan)
+static bool cachefiles_open_file(struct cachefiles_object *object,
+ struct dentry *dentry)
{
struct cachefiles_cache *cache = object->volume->cache;
- struct dentry *dentry;
struct file *file;
struct path path;
int ret;

- _enter("%pd %s", fan, object->d_name);
+ _enter("%pd", dentry);

- dentry = lookup_positive_unlocked(object->d_name, fan, object->d_name_len);
- trace_cachefiles_lookup(object, dentry);
- if (dentry == ERR_PTR(-ENOENT)) {
- set_bit(CACHEFILES_OBJECT_IS_NEW, &object->flags);
- fscache_cookie_lookup_negative(object->cookie);
-
- ret = cachefiles_has_space(cache, 1, 0);
- if (ret < 0)
- goto error;
+ if (!cachefiles_mark_inode_in_use(object, dentry))
+ return false;

- file = cachefiles_create_tmpfile(object);
- if (IS_ERR(file)) {
- ret = PTR_ERR(file);
- goto error;
- }
+ /* We need to open a file interface onto a data file now as we can't do
+ * it on demand because writeback called from do_exit() sees
+ * current->fs == NULL - which breaks d_path() called from ext4 open.
+ */
+ path.mnt = cache->mnt;
+ path.dentry = dentry;
+ file = open_with_fake_path(&path, O_RDWR | O_LARGEFILE | O_DIRECT,
+ d_backing_inode(dentry), cache->cache_cred);
+ if (IS_ERR(file))
+ goto error;

- set_bit(FSCACHE_COOKIE_NEEDS_UPDATE, &object->cookie->flags);
- set_bit(CACHEFILES_OBJECT_USING_TMPFILE, &object->flags);
- _debug("create -> %pD{ino=%lu}", file, file_inode(file)->i_ino);
- goto out;
+ if (unlikely(!file->f_op->read_iter) ||
+ unlikely(!file->f_op->write_iter)) {
+ pr_notice("Cache does not support read_iter and write_iter\n");
+ goto error_fput;
}
+ _debug("file -> %pd positive", dentry);

- if (IS_ERR(dentry)) {
- ret = PTR_ERR(dentry);
- goto error;
- }
+ ret = cachefiles_check_auxdata(object, file);
+ if (ret < 0)
+ goto check_failed;

- if (!d_is_reg(dentry)) {
- pr_err("%pd is not a file\n", dentry);
- dput(dentry);
- ret = -EIO;
- goto error;
- } else {
- clear_bit(CACHEFILES_OBJECT_IS_NEW, &object->flags);
+ object->file = file;

- /* We need to open a file interface onto a data file now as we
- * can't do it on demand because writeback called from
- * do_exit() sees current->fs == NULL - which breaks d_path()
- * called from ext4 open.
- */
- path.mnt = cache->mnt;
- path.dentry = dentry;
- file = open_with_fake_path(&path, O_RDWR | O_LARGEFILE | O_DIRECT,
- d_backing_inode(dentry), cache->cache_cred);
+ /* Always update the atime on an object we've just looked up (this is
+ * used to keep track of culling, and atimes are only updated by read,
+ * write and readdir but not lookup or open).
+ */
+ touch_atime(&file->f_path);
+ dput(dentry);
+ return true;
+
+check_failed:
+ fscache_cookie_lookup_negative(object->cookie);
+ cachefiles_unmark_inode_in_use(object, file);
+ cachefiles_mark_object_inactive(object, file);
+ if (ret == -ESTALE) {
+ fput(file);
dput(dentry);
- if (IS_ERR(file)) {
- ret = PTR_ERR(file);
- goto error;
- }
- if (unlikely(!file->f_op->read_iter) ||
- unlikely(!file->f_op->write_iter)) {
- pr_notice("Cache does not support read_iter and write_iter\n");
- ret = -EIO;
- goto error_fput;
- }
- _debug("file -> %pd positive", dentry);
+ return cachefiles_create_file(object);
}
-
-out:
- object->file = file;
- return 0;
error_fput:
fput(file);
error:
- return ret;
+ dput(dentry);
+ return false;
}

/*
* walk from the parent object to the child object through the backing
* filesystem, creating directories as we go
*/
-bool cachefiles_walk_to_object(struct cachefiles_object *object)
+bool cachefiles_look_up_object(struct cachefiles_object *object)
{
- struct cachefiles_volume *volume = object->cookie->volume->cache_priv;
- struct dentry *fan;
+ struct cachefiles_volume *volume = object->volume;
+ struct dentry *dentry, *fan = volume->fanout[(u8)object->key_hash];
int ret;

_enter("OBJ%x,%s,", object->debug_id, object->d_name);

-lookup_again:
- /* Open path "cache/vol/fanout/file". */
- fan = volume->fanout[(u8)object->key_hash];
- ret = cachefiles_open_file(object, fan);
- if (ret < 0)
- goto lookup_error;
+ /* Look up path "cache/vol/fanout/file". */
+ dentry = lookup_positive_unlocked(object->d_name, fan, object->d_name_len);
+ trace_cachefiles_lookup(object, dentry);
+ if (IS_ERR(dentry)) {
+ if (dentry == ERR_PTR(-ENOENT))
+ goto new_file;
+ if (dentry == ERR_PTR(-EIO))
+ cachefiles_io_error_obj(object, "Lookup failed");
+ return false;
+ }

- if (!test_bit(CACHEFILES_OBJECT_IS_NEW, &object->flags)) {
- ret = cachefiles_check_open_object(object, fan);
+ if (!d_is_reg(dentry)) {
+ pr_err("%pd is not a file\n", dentry);
+ inode_lock_nested(d_inode(fan), I_MUTEX_PARENT);
+ ret = cachefiles_bury_object(volume->cache, object, fan, dentry,
+ FSCACHE_OBJECT_IS_WEIRD);
+ dput(dentry);
if (ret < 0)
- goto check_error;
- } else {
- ret = -EBUSY;
- if (!cachefiles_mark_inode_in_use(object))
- goto check_error;
+ return false;
+ goto new_file;
}

- clear_bit(CACHEFILES_OBJECT_IS_NEW, &object->flags);
+ if (!cachefiles_open_file(object, dentry))
+ return false;
+
_leave(" = t [%lu]", file_inode(object->file)->i_ino);
return true;

-check_error:
- fput(object->file);
- object->file = NULL;
- if (ret == -ESTALE)
- goto lookup_again;
-lookup_error:
- if (ret == -EIO)
- cachefiles_io_error_obj(object, "Lookup failed");
- return false;
+new_file:
+ fscache_cookie_lookup_negative(object->cookie);
+ return cachefiles_create_file(object);
}

/*
@@ -709,6 +701,11 @@ struct file *cachefiles_create_tmpfile(struct cachefiles_object *object)

trace_cachefiles_tmpfile(object, d_backing_inode(path.dentry));

+ if (!cachefiles_mark_inode_in_use(object, path.dentry)) {
+ file = ERR_PTR(-EBUSY);
+ goto out_dput;
+ }
+
if (ni_size > 0) {
trace_cachefiles_trunc(object, d_backing_inode(path.dentry), 0, ni_size,
cachefiles_trunc_expand_tmpfile);
@@ -757,6 +754,24 @@ bool cachefiles_commit_tmpfile(struct cachefiles_cache *cache,
goto out_unlock;
}

+ if (!d_is_negative(dentry)) {
+ if (d_backing_inode(dentry) == file_inode(object->file)) {
+ success = true;
+ goto out_dput;
+ }
+
+ ret = cachefiles_unlink(object, fan, dentry, FSCACHE_OBJECT_IS_STALE);
+ if (ret < 0)
+ goto out_dput;
+
+ dput(dentry);
+ dentry = lookup_one_len(object->d_name, fan, object->d_name_len);
+ if (IS_ERR(dentry)) {
+ _debug("lookup fail %ld", PTR_ERR(dentry));
+ goto out_unlock;
+ }
+ }
+
ret = vfs_link(object->file->f_path.dentry, &init_user_ns,
d_inode(fan), dentry, NULL);
if (ret < 0) {
@@ -770,6 +785,7 @@ bool cachefiles_commit_tmpfile(struct cachefiles_cache *cache,
success = true;
}

+out_dput:
dput(dentry);
out_unlock:
inode_unlock(d_inode(fan));
diff --git a/fs/cachefiles/xattr.c b/fs/cachefiles/xattr.c
index b77bbb6c4a17..50b2a4588946 100644
--- a/fs/cachefiles/xattr.c
+++ b/fs/cachefiles/xattr.c
@@ -74,10 +74,10 @@ int cachefiles_set_object_xattr(struct cachefiles_object *object)
/*
* check the consistency between the backing cache and the FS-Cache cookie
*/
-int cachefiles_check_auxdata(struct cachefiles_object *object)
+int cachefiles_check_auxdata(struct cachefiles_object *object, struct file *file)
{
struct cachefiles_xattr *buf;
- struct dentry *dentry = object->file->f_path.dentry;
+ struct dentry *dentry = file->f_path.dentry;
unsigned int len = object->cookie->aux_len, tlen;
const void *p = fscache_get_aux(object->cookie);
enum cachefiles_coherency_trace why;
@@ -105,7 +105,7 @@ int cachefiles_check_auxdata(struct cachefiles_object *object)
ret = 0;
}

- trace_cachefiles_coherency(object, file_inode(object->file)->i_ino, 0, why);
+ trace_cachefiles_coherency(object, file_inode(file)->i_ino, 0, why);
kfree(buf);
return ret;
}
diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
index c0632ee8cf69..a7b31b248f2d 100644
--- a/include/trace/events/cachefiles.h
+++ b/include/trace/events/cachefiles.h
@@ -35,6 +35,7 @@ enum cachefiles_obj_ref_trace {

enum fscache_why_object_killed {
FSCACHE_OBJECT_IS_STALE,
+ FSCACHE_OBJECT_IS_WEIRD,
FSCACHE_OBJECT_INVALIDATED,
FSCACHE_OBJECT_NO_SPACE,
FSCACHE_OBJECT_WAS_RETIRED,
@@ -78,6 +79,7 @@ enum cachefiles_prepare_read_trace {
*/
#define cachefiles_obj_kill_traces \
EM(FSCACHE_OBJECT_IS_STALE, "stale") \
+ EM(FSCACHE_OBJECT_IS_WEIRD, "weird") \
EM(FSCACHE_OBJECT_INVALIDATED, "inval") \
EM(FSCACHE_OBJECT_NO_SPACE, "no_space") \
EM(FSCACHE_OBJECT_WAS_RETIRED, "was_retired") \