On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote: > From: NeilBrown <[email protected]> > > 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. > > Signed-off-by: NeilBrown <[email protected]> > --- > fs/cachefiles/namei.c | 76 ++++++++----------------------------------- > 1 file changed, 14 insertions(+), 62 deletions(-) > > diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c > index e5ec90dccc27..3af42ec78411 100644 > --- 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,36 @@ int cachefiles_bury_object(struct cachefiles_cache > *cache, > (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; > - } > - > - if (trap == rep) { > - unlock_rename(cache->graveyard, dir); > - cachefiles_io_error(cache, "May not make directory loop"); > + 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) { > + cachefiles_io_error(cache, "Cannot lock/lookup in graveyard"); > 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 +342,6 @@ int cachefiles_bury_object(struct cachefiles_cache > *cache, > 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 +355,7 @@ int cachefiles_bury_object(struct cachefiles_cache *cache, > } > > __cachefiles_unmark_inode_in_use(object, d_inode(rep)); > - unlock_rename(cache->graveyard, dir); > - dput(grave); > + end_renaming(&rd); > _leave(" = 0"); > return 0; > }
Reviewed-by: Jeff Layton <[email protected]>
