From: NeilBrown 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 --- 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; } -- 2.50.0.107.gf914562f5916.dirty