cachefiles: change cachefiles_bury_object to use start_renaming_dentry()

Rather then using lock_rename() and lookup_one() etc we can use
the new start_renaming_dentry().  This is part of centralising dir
locking and lookup so that locking rules can be changed.

Some error check are removed as not necessary.  Checks for rep being a
non-dir or IS_DEADDIR and the check that ->graveyard is still a
directory only provide slightly more informative errors and have been
dropped.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260224222542.3458677-11-neilb@ownmail.net
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index e5ec90dccc27f71dc19219f7632f3e48eaf51545..c464c72a51cb390a0a7c715c7202dde4812e1e28 100644 (file)
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -270,7 +270,8 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
                           struct dentry *rep,
                           enum fscache_why_object_killed why)
 {
-       struct dentry *grave, *trap;
+       struct dentry *grave;
+       struct renamedata rd = {};
        struct path path, path_to_graveyard;
        char nbuffer[8 + 8 + 1];
        int ret;
@@ -302,77 +303,55 @@ try_again:
                (uint32_t) ktime_get_real_seconds(),
                (uint32_t) atomic_inc_return(&cache->gravecounter));
 
-       /* do the multiway lock magic */
-       trap = lock_rename(cache->graveyard, dir);
-       if (IS_ERR(trap))
-               return PTR_ERR(trap);
-
-       /* do some checks before getting the grave dentry */
-       if (rep->d_parent != dir || IS_DEADDIR(d_inode(rep))) {
-               /* the entry was probably culled when we dropped the parent dir
-                * lock */
-               unlock_rename(cache->graveyard, dir);
-               _leave(" = 0 [culled?]");
-               return 0;
-       }
-
-       if (!d_can_lookup(cache->graveyard)) {
-               unlock_rename(cache->graveyard, dir);
-               cachefiles_io_error(cache, "Graveyard no longer a directory");
-               return -EIO;
-       }
+       rd.mnt_idmap = &nop_mnt_idmap;
+       rd.old_parent = dir;
+       rd.new_parent = cache->graveyard;
+       rd.flags = 0;
+       ret = start_renaming_dentry(&rd, 0, rep, &QSTR(nbuffer));
+       if (ret) {
+               /* Some errors aren't fatal */
+               if (ret == -EXDEV)
+                       /* double-lock failed */
+                       return ret;
+               if (d_unhashed(rep) || rep->d_parent != dir || IS_DEADDIR(d_inode(rep))) {
+                       /* the entry was probably culled when we dropped the parent dir
+                        * lock */
+                       _leave(" = 0 [culled?]");
+                       return 0;
+               }
+               if (ret == -EINVAL || ret == -ENOTEMPTY) {
+                       cachefiles_io_error(cache, "May not make directory loop");
+                       return -EIO;
+               }
+               if (ret == -ENOMEM) {
+                       _leave(" = -ENOMEM");
+                       return -ENOMEM;
+               }
 
-       if (trap == rep) {
-               unlock_rename(cache->graveyard, dir);
-               cachefiles_io_error(cache, "May not make directory loop");
+               cachefiles_io_error(cache, "Lookup error %d", ret);
                return -EIO;
        }
 
        if (d_mountpoint(rep)) {
-               unlock_rename(cache->graveyard, dir);
+               end_renaming(&rd);
                cachefiles_io_error(cache, "Mountpoint in cache");
                return -EIO;
        }
 
-       grave = lookup_one(&nop_mnt_idmap, &QSTR(nbuffer), cache->graveyard);
-       if (IS_ERR(grave)) {
-               unlock_rename(cache->graveyard, dir);
-               trace_cachefiles_vfs_error(object, d_inode(cache->graveyard),
-                                          PTR_ERR(grave),
-                                          cachefiles_trace_lookup_error);
-
-               if (PTR_ERR(grave) == -ENOMEM) {
-                       _leave(" = -ENOMEM");
-                       return -ENOMEM;
-               }
-
-               cachefiles_io_error(cache, "Lookup error %ld", PTR_ERR(grave));
-               return -EIO;
-       }
-
+       grave = rd.new_dentry;
        if (d_is_positive(grave)) {
-               unlock_rename(cache->graveyard, dir);
-               dput(grave);
+               end_renaming(&rd);
                grave = NULL;
                cond_resched();
                goto try_again;
        }
 
        if (d_mountpoint(grave)) {
-               unlock_rename(cache->graveyard, dir);
-               dput(grave);
+               end_renaming(&rd);
                cachefiles_io_error(cache, "Mountpoint in graveyard");
                return -EIO;
        }
 
-       /* target should not be an ancestor of source */
-       if (trap == grave) {
-               unlock_rename(cache->graveyard, dir);
-               dput(grave);
-               cachefiles_io_error(cache, "May not make directory loop");
-               return -EIO;
-       }
-
        /* attempt the rename */
        path.mnt = cache->mnt;
        path.dentry = dir;
@@ -382,13 +361,6 @@ try_again:
        if (ret < 0) {
                cachefiles_io_error(cache, "Rename security error %d", ret);
        } else {
-               struct renamedata rd = {
-                       .mnt_idmap      = &nop_mnt_idmap,
-                       .old_parent     = dir,
-                       .old_dentry     = rep,
-                       .new_parent     = cache->graveyard,
-                       .new_dentry     = grave,
-               };
                trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
                ret = cachefiles_inject_read_error();
                if (ret == 0)
@@ -402,8 +374,7 @@ try_again:
        }
 
        __cachefiles_unmark_inode_in_use(object, d_inode(rep));
-       unlock_rename(cache->graveyard, dir);
-       dput(grave);
+       end_renaming(&rd);
        _leave(" = 0");
        return 0;
 }